fix(chart): Hide Baidu Maps and Amap from the document and Repair the chart document #3867
Conversation
WalkthroughCommented out two map chart entries in the PC demo menu; changed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/sites/demos/pc/menus.js (1)
350-351: Hiding Baidu/Autonavi chart menu items is fine; consider adding a brief rationale commentCommenting out these two entries cleanly removes the Baidu/Autonavi map charts from the Chart menu without affecting surrounding items or structure, so the behavior change is safe.
To help future maintainers understand why these routes are hidden (and that native maps are preferred now), consider adding a short note similar to other hidden items in this file, for example:
- // { 'nameCn': '百度地图', 'name': 'BaiduMap Chart', 'key': 'chart-baidu-map' }, - // { 'nameCn': '高德地图', 'name': 'Autonavi Chart', 'key': 'chart-autonavi-map' }, + // { 'nameCn': '百度地图', 'name': 'BaiduMap Chart', 'key': 'chart-baidu-map' }, // 已下线，推荐使用原生地图能力 + // { 'nameCn': '高德地图', 'name': 'Autonavi Chart', 'key': 'chart-autonavi-map' }, // 已下线，推荐使用原生地图能力This keeps the intent aligned with the PR description and avoids confusion later.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/sites/src/components/api-docs.vue (1)
39-39: Critical: Inconsistent prop name will break highlighting.Line 39 still uses the old prop name
types="ts", but the AsyncHighlight component now expectstype. This inconsistency will cause the prop to be ignored, resulting in incorrect or missing syntax highlighting for dependent types.Apply this diff to fix the prop name:
<async-highlight v-for="(k, i) in row.depTypes" :key="i" :code="props.currentJson.types[k]?.code" - types="ts" + type="ts" ></async-highlight>
🧹 Nitpick comments (1)
examples/sites/src/components/async-highlight.vue (1)
23-25: Consider adding a default value for consistency.The new
typeprop lacks a default value. While this follows the pattern of thefilenameprop, addingdefault: ''would make the behavior more explicit and prevent potential issues if the prop is undefined.Apply this diff to add a default value:
type: { - type: String + type: String, + default: '' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/sites/src/components/api-docs.vue(1 hunks)examples/sites/src/components/async-highlight.vue(2 hunks)
🔇 Additional comments (2)
examples/sites/src/components/async-highlight.vue (1)
38-39: LGTM! Chart code bypass is correctly implemented.The new conditional branch properly handles chart code by skipping syntax highlighting when
type === 'chart'. This aligns with the PR's objective to handle chart content differently.examples/sites/src/components/api-docs.vue (1)
32-32: LGTM! Prop update correctly aligns with AsyncHighlight changes.The change from
:typesto:typewith updated value mapping ('chart' vs empty string) correctly aligns with the new prop and logic in the AsyncHighlight component.
Davont
changed the title
Davont
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/sites/src/components/api-docs.vue(1 hunks)
🔇 Additional comments (1)
examples/sites/src/components/api-docs.vue (1)
35-40: LGTM! Consistent prop rename.The change from
types="ts"totype="ts"is consistent with the AsyncHighlight API update and maintains the TypeScript highlighting for dependency types.
| <async-highlight | ||
| v-if="row.code" | ||
| :code="row.code.trim()" | ||
| :types="props.chartCode ? 'html' : 'ts'" | ||
| :type="props.chartCode ? 'chart' : ''" | ||
| ></async-highlight> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat examples/sites/src/components/async-highlight.vueRepository: opentiny/tiny-vue
Length of output: 2689
🏁 Script executed:
git log --oneline -20 examples/sites/src/components/async-highlight.vueRepository: opentiny/tiny-vue
Length of output: 151
🏁 Script executed:
fd async-highlight examples/sites/src/Repository: opentiny/tiny-vue
Length of output: 110
🏁 Script executed:
git log --oneline -10 examples/sites/src/components/api-docs.vueRepository: opentiny/tiny-vue
Length of output: 151
🏁 Script executed:
git show --stat HEADRepository: opentiny/tiny-vue
Length of output: 50374
The type prop passed to AsyncHighlight does not exist in the component definition.
AsyncHighlight only accepts code and filename props. The :type="props.chartCode ? 'chart' : ''" binding on lines 32 and similar usage elsewhere will be silently ignored. The component uses the filename prop to determine syntax highlighting language (checks if it ends with .vue for HTML highlighting, otherwise uses auto-detect). Either update AsyncHighlight to accept and use a type prop, or remove the type binding from api-docs.vue and rely on the filename prop instead.
🤖 Prompt for AI Agents
In examples/sites/src/components/api-docs.vue around lines 29–33, the
AsyncHighlight component is being passed a non-existent type prop
(:type="props.chartCode ? 'chart' : ''"); AsyncHighlight only accepts code and
filename so this binding is ignored. Fix by removing the :type binding from the
AsyncHighlight usage and, if you need chart-specific highlighting, populate or
derive the filename prop (e.g., set filename to a .vue/.js/.chart-like value or
compute filename when props.chartCode is true) so the component can select the
correct language via its existing filename logic.
1 participant
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.