feat(content-nav): add content navigation component#322
Open
SonyLeo wants to merge 5 commits intoopentiny:developfrom
Open
feat(content-nav): add content navigation component#322SonyLeo wants to merge 5 commits intoopentiny:developfrom
SonyLeo wants to merge 5 commits intoopentiny:developfrom
Conversation
WalkthroughThis pull request introduces a complete content navigation component system ( Changes
Sequence DiagramsequenceDiagram
participant User
participant ContentNav as ContentNav<br/>(Main)
participant NavController as useNavController<br/>(State)
participant ActiveSync as useActiveSync<br/>(Scroll Sync)
participant Interactions as useOverlayInteractions<br/>(Keyboard/Mouse)
participant TargetFeedback as useTargetFeedback<br/>(Animations)
User->>ContentNav: Scroll viewport
activate ContentNav
ContentNav->>ActiveSync: sync()
activate ActiveSync
ActiveSync->>ActiveSync: Read scroll position,<br/>resolve anchor positions
ActiveSync->>NavController: Update activeId
deactivate ActiveSync
deactivate ContentNav
User->>Interactions: Press arrow key / click item
activate Interactions
Interactions->>NavController: setQuery() or<br/>handleNavigationKeydown()
activate NavController
NavController->>NavController: Filter items by search,<br/>compute highlighted index
deactivate NavController
deactivate Interactions
User->>Interactions: Press Enter / click to select
activate Interactions
Interactions->>ContentNav: handleSelect(itemId)
activate ContentNav
ContentNav->>ActiveSync: scrollTo(id)
activate ActiveSync
ActiveSync->>ActiveSync: Calculate target offset,<br/>smooth scroll
deactivate ActiveSync
ContentNav->>TargetFeedback: activate(id)
activate TargetFeedback
TargetFeedback->>TargetFeedback: Apply animation class,<br/>schedule removal
deactivate TargetFeedback
ContentNav->>ContentNav: Emit select & activate
deactivate ContentNav
deactivate Interactions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
|
✅ Preview build completed successfully!
|
Contributor
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@f204920 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@f204920 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@f204920 commit: f204920 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/components/src/content-nav/composables/useFloatingOffset.ts (2)
31-48: Consider reactivity limitations ofresizeTargets.The
resizeTargetscomputed callsgetFloatingNodes()which queries the DOM viafirstElementChildandquerySelector. Since these DOM queries aren't reactive dependencies, the computed won't automatically update if the overlay's internal DOM structure changes after initial render.This is likely fine if the overlay structure is stable once mounted, but worth noting if dynamic slot content could affect the measured elements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/composables/useFloatingOffset.ts` around lines 31 - 48, The computed resizeTargets relies on getFloatingNodes() which performs DOM queries (firstElementChild/querySelector) so it won't react to later DOM changes; to fix, make the dependency explicit by tracking a reactive ref that changes when the overlay's internal DOM updates (e.g., expose or watch the measured/floating/host refs from getFloatingNodes or emit an updateRef), or attach a MutationObserver inside useFloatingOffset (or getFloatingNodes) to update a local reactive token so resizeTargets recomputes; update references to resizeTargets/getFloatingNodes accordingly so dynamic slot content triggers recomputation.
73-74: Consider extracting the padding constant.The
24pixel padding appears twice. A named constant would clarify intent and simplify future adjustments.♻️ Optional refactor
+const FLOATING_PADDING = 24 + function sync() { // ... - const minTop = containerTop + 24 - const maxTop = Math.max(minTop, containerBottom - floatingHeight - 24) + const minTop = containerTop + FLOATING_PADDING + const maxTop = Math.max(minTop, containerBottom - floatingHeight - FLOATING_PADDING)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/composables/useFloatingOffset.ts` around lines 73 - 74, Extract the repeated numeric padding (24) into a named constant (e.g., const FLOATING_PADDING = 24) at the top of useFloatingOffset.ts and replace the two literal occurrences in the calculations for minTop and maxTop (where minTop = containerTop + 24 and maxTop = Math.max(minTop, containerBottom - floatingHeight - 24)) with that constant to clarify intent and make future adjustments easier.packages/components/src/content-nav/components/ContentNavSearch.vue (1)
15-16: Consider aligningaria-labelwithplaceholderfor consistency.When
options?.placeholderis customized, thearia-labelfalls back to a different default ("Search content navigation") than the placeholder ("Search"). This could create a mismatch where screen reader users hear different text than what's visually displayed.♻️ Optional alignment
- :placeholder="props.options?.placeholder ?? 'Search'" - :aria-label="props.options?.placeholder ?? 'Search content navigation'" + :placeholder="props.options?.placeholder ?? 'Search'" + :aria-label="props.options?.ariaLabel ?? props.options?.placeholder ?? 'Search'"Alternatively, if
ariaLabelisn't in the options type, you could use the same fallback for both or derive a more descriptive label from the placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/components/ContentNavSearch.vue` around lines 15 - 16, The aria-label fallback is inconsistent with the placeholder in ContentNavSearch.vue — update the binding for aria-label (currently using props.options?.placeholder ?? 'Search content navigation') to match the placeholder fallback (props.options?.placeholder ?? 'Search') or add an explicit options.ariaLabel and use props.options?.ariaLabel ?? props.options?.placeholder ?? 'Search' so screen readers always hear the same text shown in the placeholder; adjust the template binding accordingly (reference props.options?.placeholder and aria-label).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/src/content-nav/composables/useActiveSync.ts`:
- Around line 54-58: sync() and scrollTo() currently return early when
options.container.value is falsy, preventing auto-activation and programmatic
scrolling at the document level; update both functions (sync() and scrollTo())
to use a document-level fallback when options.container.value is missing (e.g.,
treat missing container as window/document.documentElement for bounding/scroll
calculations) so targets resolved by index.vue still auto-activate and scroll
correctly; ensure you reference options.container in sync() and scrollTo() and
branch to the fallback logic for calculating target positions and performing
scroll behavior instead of bailing out.
In `@packages/components/src/content-nav/composables/useNavState.ts`:
- Around line 27-35: The matcher currently treats an empty array as a truthy
match, so update the filter logic in useNavState (around matcher.value and the
segments variable) to treat empty arrays as non-matches: when keyword is
present, return null if segments is falsy OR segments.length === 0, and only
call ensureContentNavSegments(item, segments) after confirming segments is a
non-empty array.
- Around line 16-18: The computed expanded currently chooses state based on
isManualExpandTrigger, which ties controlled/uncontrolled ownership to
expandTrigger and causes manual+uncontrolled to default false and
hover+controlled to ignore options.expanded; change expanded (and the similar
block at lines ~69-72) to derive value first from the external control if
provided, otherwise fall back to internal state—i.e. use options.expanded.value
(when defined) else localExpanded.value—so update the computed using the symbols
expanded, isManualExpandTrigger/expandTrigger, options.expanded, and
localExpanded to respect controlled vs uncontrolled regardless of trigger mode.
In `@packages/components/src/content-nav/composables/useOverlayInteractions.ts`:
- Around line 25-26: The selector-building using escapedId is unsafe when
CSS.escape is missing; update the focus logic in useOverlayInteractions (the
line using
options.overlay.value?.navEl?.querySelector<HTMLElement>(`[data-item-id="${escapedId}"]`)?.focus())
to use a safe fallback: when CSS.escape is unavailable, call
navEl.querySelectorAll('[data-item-id]'), convert to an array (Array.from), find
the element whose dataset.itemId (or dataset['itemId']) strictly equals the raw
id, and call focus() on that element if found; keep the existing path that uses
CSS.escape when available.
---
Nitpick comments:
In `@packages/components/src/content-nav/components/ContentNavSearch.vue`:
- Around line 15-16: The aria-label fallback is inconsistent with the
placeholder in ContentNavSearch.vue — update the binding for aria-label
(currently using props.options?.placeholder ?? 'Search content navigation') to
match the placeholder fallback (props.options?.placeholder ?? 'Search') or add
an explicit options.ariaLabel and use props.options?.ariaLabel ??
props.options?.placeholder ?? 'Search' so screen readers always hear the same
text shown in the placeholder; adjust the template binding accordingly
(reference props.options?.placeholder and aria-label).
In `@packages/components/src/content-nav/composables/useFloatingOffset.ts`:
- Around line 31-48: The computed resizeTargets relies on getFloatingNodes()
which performs DOM queries (firstElementChild/querySelector) so it won't react
to later DOM changes; to fix, make the dependency explicit by tracking a
reactive ref that changes when the overlay's internal DOM updates (e.g., expose
or watch the measured/floating/host refs from getFloatingNodes or emit an
updateRef), or attach a MutationObserver inside useFloatingOffset (or
getFloatingNodes) to update a local reactive token so resizeTargets recomputes;
update references to resizeTargets/getFloatingNodes accordingly so dynamic slot
content triggers recomputation.
- Around line 73-74: Extract the repeated numeric padding (24) into a named
constant (e.g., const FLOATING_PADDING = 24) at the top of useFloatingOffset.ts
and replace the two literal occurrences in the calculations for minTop and
maxTop (where minTop = containerTop + 24 and maxTop = Math.max(minTop,
containerBottom - floatingHeight - 24)) with that constant to clarify intent and
make future adjustments easier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a7e5993e-a506-4def-9c28-295b2300f5a8
📒 Files selected for processing (20)
packages/components/src/bubble/composables/useBubbleBoxRenderer.tspackages/components/src/bubble/index.type.tspackages/components/src/content-nav/components/ContentNavItem.vuepackages/components/src/content-nav/components/ContentNavList.vuepackages/components/src/content-nav/components/ContentNavOverlay.vuepackages/components/src/content-nav/components/ContentNavSearch.vuepackages/components/src/content-nav/composables/index.tspackages/components/src/content-nav/composables/useActiveSync.tspackages/components/src/content-nav/composables/useFloatingOffset.tspackages/components/src/content-nav/composables/useNavState.tspackages/components/src/content-nav/composables/useOverlayInteractions.tspackages/components/src/content-nav/defaults.tspackages/components/src/content-nav/index.tspackages/components/src/content-nav/index.type.tspackages/components/src/content-nav/index.vuepackages/components/src/content-nav/internal.type.tspackages/components/src/content-nav/target.tspackages/components/src/index.tspackages/components/src/styles/components/content-nav.lesspackages/components/src/styles/components/index.css
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/components/src/content-nav/index.vue (1)
45-45: Redundant computed wrapper.
itemsfromtoRefs(props)is already aRef<ContentNavItem[]>. Wrapping it incomputed(() => items.value)creates an unnecessary intermediate reactive layer.🛠️ Suggested fix
-const itemsRef = computed(() => items.value) +const itemsRef = items🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/index.vue` at line 45, The computed wrapper itemsRef is redundant because items (from toRefs(props)) is already a Ref<ContentNavItem[]>; remove the computed(() => items.value) declaration (itemsRef) and replace its usages with the original items ref (or items.value where a raw array is needed), updating any references to itemsRef in the component (template bindings, watchers, methods) to use items so you eliminate the unnecessary intermediate reactive layer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/src/content-nav/composables/useActiveSync.ts`:
- Around line 126-133: The watcher on options.activeId?.value in
useActiveSync.ts ignores transitions to undefined, leaving localActiveId stale;
change the watcher callback to always assign localActiveId.value = value (or
explicitly set localActiveId.value = undefined when value === undefined) so that
localActiveId is synced even when options.activeId becomes undefined, and keep
the watcher signature using watch(() => options.activeId?.value, (value) => {
... }) referencing options.activeId and localActiveId.
In `@packages/components/src/content-nav/target.ts`:
- Around line 7-8: Replace the CSS.escape + attribute selector branch with the
dataset-comparison fallback so we never construct a quoted attribute selector
containing unescaped quotes; specifically, remove or bypass the block that does
root.querySelector<HTMLElement>(`[${attribute}="${CSS.escape(id)}"]`) and always
use the dataset comparison approach (the same logic used in the existing
fallback path that reads element.dataset[...] or compares getAttribute values
safely) to locate the element by id/attribute, referencing the same variables
id, attribute, and root used in target.ts.
---
Nitpick comments:
In `@packages/components/src/content-nav/index.vue`:
- Line 45: The computed wrapper itemsRef is redundant because items (from
toRefs(props)) is already a Ref<ContentNavItem[]>; remove the computed(() =>
items.value) declaration (itemsRef) and replace its usages with the original
items ref (or items.value where a raw array is needed), updating any references
to itemsRef in the component (template bindings, watchers, methods) to use items
so you eliminate the unnecessary intermediate reactive layer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c89e5dc3-a07c-4b35-9bec-77a2cb35fd42
📒 Files selected for processing (7)
packages/components/src/content-nav/composables/useActiveSync.tspackages/components/src/content-nav/composables/useNavState.tspackages/components/src/content-nav/composables/useOverlayInteractions.tspackages/components/src/content-nav/defaults.tspackages/components/src/content-nav/index.vuepackages/components/src/content-nav/scroll.tspackages/components/src/content-nav/target.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/src/content-nav/composables/useOverlayInteractions.ts
- packages/components/src/content-nav/composables/useNavState.ts
SonyLeo
force-pushed
the
feat/content-nav-comp
branch
from
e62885f to
ad5db97
Compare
gene9831
requested changes
Apr 20, 2026
Collaborator
gene9831
left a comment
gene9831
left a comment
There was a problem hiding this comment.
如果第一次展开导航组件，并且悬停到未超长的文本项上，也会显示toolTip
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/components/src/content-nav/composables/useNavController.ts (1)
15-18:⚠️ Potential issue | 🟠 MajorControlled/uncontrolled
expandedstill tied toexpandTrigger— not addressed from previous review.Line 17 still returns
options.expanded.value ?? falseonly whenexpandTrigger === 'manual', and Line 71 only writeslocalExpandedwhen not manual. This means:
expandTrigger: 'manual'+ uncontrolled →expandedis stuck atfalse(no writer).expandTrigger: 'hover'+ controlled → externaloptions.expandedis ignored.Ownership should be decided by whether a controlled value is provided, independent of trigger mode — the same pattern already used by
setQueryat Line 83.💡 Proposed fix
- const expanded = computed(() => - isManualExpandTrigger.value ? (options.expanded.value ?? false) : localExpanded.value, - ) + const expanded = computed(() => options.expanded.value ?? localExpanded.value) @@ function setExpanded(value: boolean) { - if (!isManualExpandTrigger.value) { + if (options.expanded.value === undefined) { localExpanded.value = value } options.onUpdateExpanded?.(value)Also applies to: 70-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/composables/useNavController.ts` around lines 15 - 18, The computed ownership for expanded is wrong: decide control based on whether options.expanded is provided, not on expandTrigger; change the logic in isManualExpandTrigger/expanded so expanded returns options.expanded.value when options.expanded is defined (controlled), otherwise uses localExpanded (uncontrolled), and update any writers so setLocalExpanded (or localExpanded mutation) is used when uncontrolled regardless of expandTrigger — mirror the ownership pattern used by setQuery to detect controlled vs uncontrolled and apply it to the expanded computed and its writers.
🧹 Nitpick comments (4)
packages/components/src/content-nav/index.vue (2)
66-66: Minor:itemsRefis redundant.
itemsfromtoRefs(props)is already aRef<ContentNavItem[]>; wrapping it in anothercomputedonly adds indirection. You can passitemsdirectly touseActiveSync,useNavController, and useitems.valueinshouldRender/handleSelect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/index.vue` at line 66, Remove the redundant computed wrapper itemsRef and use the original Ref from toRefs(props) directly: replace usages of itemsRef with items (pass items into useActiveSync and useNavController, and use items.value inside shouldRender and handleSelect) so you eliminate the extra indirection introduced by const itemsRef = computed(() => items.value).
171-178: Edge case: id-list key viajoin(',')can false-positive / false-negative.Joining with
','means items[{id:'a,b'}]and[{id:'a'},{id:'b'}]produce the same key, and comma-free id reorders still trigger correctly. Low risk in practice, but usingJSON.stringify(items.value.map(i => i.id))or a separator that cannot appear in ids (e.g.'\u0000') avoids the ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/index.vue` around lines 171 - 178, The current watch key uses itemsRef.value.map(item => item.id).join(',') which can collide for ids containing commas or different shapes producing the same string; update the watch key to a collision-safe serialization such as JSON.stringify(itemsRef.value.map(i => i.id)) (or use a non-possible separator like '\u0000') while keeping the same callback (active.clearPendingScroll and scheduleMeasure) and the { immediate: true } option so the reactive behavior of the watch, the referenced itemsRef, and the actions in the callback remain unchanged.packages/components/src/styles/components/content-nav.less (1)
49-77: Honorprefers-reduced-motionfor target-flash/outline animations.Users with reduced-motion preferences will still see the flash/outline pulse on navigation. The
useActiveSynccomposable already respects this for scroll; the CSS target feedback should match.♻️ Suggested addition
`@keyframes` tr-content-nav-target-outline { 0% { box-shadow: 0 0 0 2px color-mix(in srgb, var(--tr-color-primary) 42%, transparent); } 100% { box-shadow: none; } } + +@media (prefers-reduced-motion: reduce) { + .tr-content-nav-target--flash, + .tr-content-nav-target--outline { + animation: none; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/styles/components/content-nav.less` around lines 49 - 77, The target flash/outline animations (.tr-content-nav-target--flash, .tr-content-nav-target--outline) and their keyframes (tr-content-nav-target-flash, tr-content-nav-target-outline) do not respect users' prefers-reduced-motion setting; wrap a media query for prefers-reduced-motion: reduce that disables these animations (set animation: none and remove any transitional box-shadow/background effects) so the classes produce no motion for reduced-motion users, ensuring the CSS feedback matches the existing useActiveSync behavior.packages/components/src/content-nav/utils/target.ts (1)
6-16: Optional: use attribute-value selectors instead of scanning all matches.
querySelectorAll(selector)enumerates every element carrying the attribute before.findfilters by value, which is O(N) per lookup. A direct attribute-equality selector withCSS.escapelets the browser use its indexed lookup and removes the dataset-key duplication.♻️ Suggested refactor
-function queryByDataAttribute(root: ParentNode, selector: string, datasetKey: string, id: string) { - return Array.from(root.querySelectorAll<HTMLElement>(selector)).find((entry) => entry.dataset[datasetKey] === id) -} - -export function queryContentNavTargetById(root: ParentNode, id: string) { - return queryByDataAttribute(root, CONTENT_NAV_TARGET_SELECTOR, 'contentNavId', id) -} - -export function queryContentNavItemById(root: ParentNode, id: string) { - return queryByDataAttribute(root, CONTENT_NAV_ITEM_SELECTOR, 'itemId', id) -} +function queryByAttributeValue(root: ParentNode, attribute: string, id: string) { + return root.querySelector<HTMLElement>(`[${attribute}="${CSS.escape(id)}"]`) ?? undefined +} + +export function queryContentNavTargetById(root: ParentNode, id: string) { + return queryByAttributeValue(root, CONTENT_NAV_TARGET_ATTRIBUTE, id) +} + +export function queryContentNavItemById(root: ParentNode, id: string) { + return queryByAttributeValue(root, CONTENT_NAV_ITEM_ATTRIBUTE, id) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/src/content-nav/utils/target.ts` around lines 6 - 16, queryByDataAttribute currently enumerates all elements then filters by dataset value; change it to use a direct attribute-equality selector with CSS.escape to let the browser do the lookup. Update queryByDataAttribute(root, selector, datasetKey, id) to compute the corresponding data-attribute name by converting datasetKey from camelCase to kebab-case (e.g., contentNavId -> content-nav-id), build a selector like `${selector}[data-${attrName}="${CSS.escape(id)}"]`, and call root.querySelector<HTMLElement>(thatSelector) (removing Array.from(...).find). Keep the existing callers queryContentNavTargetById and queryContentNavItemById unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/src/content-nav/index.vue`:
- Around line 54-55: resolvedSearchOptions can be boolean true when consumers
pass search: true, so searchSlotOptions currently becomes true (because true ??
{} === true) and that boolean is passed to the options prop of
<ContentNavSearch> and the scoped-slot payload; change the computed for
searchSlotOptions to coerce non-object truthy values into an empty object (e.g.,
check typeof resolvedSearchOptions.value !== 'object' ||
resolvedSearchOptions.value === null and return {} in that case) so that
searchSlotOptions always yields a ContentNavSearchOptions object for use by the
options prop and slot.
In `@packages/components/src/styles/components/content-nav.less`:
- Around line 39-45: Stylelint is failing to parse Less's each(`@vars`, { … })
syntax; install and add postcss-less to the project and update your Stylelint
config to use it (set customSyntax: "postcss-less" in .stylelintrc.json) or
replace stylelint-config-standard-scss with stylelint-config-standard-less so
constructs like each(`@vars`, { … }) and variable interpolation
(--@{prefix}-@{key}) are recognized; update the config and rerun linting.
---
Duplicate comments:
In `@packages/components/src/content-nav/composables/useNavController.ts`:
- Around line 15-18: The computed ownership for expanded is wrong: decide
control based on whether options.expanded is provided, not on expandTrigger;
change the logic in isManualExpandTrigger/expanded so expanded returns
options.expanded.value when options.expanded is defined (controlled), otherwise
uses localExpanded (uncontrolled), and update any writers so setLocalExpanded
(or localExpanded mutation) is used when uncontrolled regardless of
expandTrigger — mirror the ownership pattern used by setQuery to detect
controlled vs uncontrolled and apply it to the expanded computed and its
writers.
---
Nitpick comments:
In `@packages/components/src/content-nav/index.vue`:
- Line 66: Remove the redundant computed wrapper itemsRef and use the original
Ref from toRefs(props) directly: replace usages of itemsRef with items (pass
items into useActiveSync and useNavController, and use items.value inside
shouldRender and handleSelect) so you eliminate the extra indirection introduced
by const itemsRef = computed(() => items.value).
- Around line 171-178: The current watch key uses itemsRef.value.map(item =>
item.id).join(',') which can collide for ids containing commas or different
shapes producing the same string; update the watch key to a collision-safe
serialization such as JSON.stringify(itemsRef.value.map(i => i.id)) (or use a
non-possible separator like '\u0000') while keeping the same callback
(active.clearPendingScroll and scheduleMeasure) and the { immediate: true }
option so the reactive behavior of the watch, the referenced itemsRef, and the
actions in the callback remain unchanged.
In `@packages/components/src/content-nav/utils/target.ts`:
- Around line 6-16: queryByDataAttribute currently enumerates all elements then
filters by dataset value; change it to use a direct attribute-equality selector
with CSS.escape to let the browser do the lookup. Update
queryByDataAttribute(root, selector, datasetKey, id) to compute the
corresponding data-attribute name by converting datasetKey from camelCase to
kebab-case (e.g., contentNavId -> content-nav-id), build a selector like
`${selector}[data-${attrName}="${CSS.escape(id)}"]`, and call
root.querySelector<HTMLElement>(thatSelector) (removing Array.from(...).find).
Keep the existing callers queryContentNavTargetById and queryContentNavItemById
unchanged.
In `@packages/components/src/styles/components/content-nav.less`:
- Around line 49-77: The target flash/outline animations
(.tr-content-nav-target--flash, .tr-content-nav-target--outline) and their
keyframes (tr-content-nav-target-flash, tr-content-nav-target-outline) do not
respect users' prefers-reduced-motion setting; wrap a media query for
prefers-reduced-motion: reduce that disables these animations (set animation:
none and remove any transitional box-shadow/background effects) so the classes
produce no motion for reduced-motion users, ensuring the CSS feedback matches
the existing useActiveSync behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c71b86d6-b177-460f-9d3d-00dd81028aff
📒 Files selected for processing (13)
packages/components/src/content-nav/components/ContentNavItem.vuepackages/components/src/content-nav/components/ContentNavList.vuepackages/components/src/content-nav/composables/index.tspackages/components/src/content-nav/composables/useActiveSync.tspackages/components/src/content-nav/composables/useNavController.tspackages/components/src/content-nav/composables/useOverlayInteractions.tspackages/components/src/content-nav/composables/useTargetFeedback.tspackages/components/src/content-nav/index.type.tspackages/components/src/content-nav/index.vuepackages/components/src/content-nav/internal.type.tspackages/components/src/content-nav/utils/scroll.tspackages/components/src/content-nav/utils/target.tspackages/components/src/styles/components/content-nav.less
✅ Files skipped from review due to trivial changes (1)
- packages/components/src/content-nav/index.type.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/components/src/content-nav/composables/index.ts
- packages/components/src/content-nav/composables/useOverlayInteractions.ts
- packages/components/src/content-nav/components/ContentNavList.vue
- packages/components/src/content-nav/components/ContentNavItem.vue
- packages/components/src/content-nav/internal.type.ts
- packages/components/src/content-nav/composables/useActiveSync.ts
…ling of search values
Collaborator
Author
该问题已经解决，详见在线示例 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更概述
本次 PR 新增
TrContentNav内容导航组件，用于长内容、结构化内容和多段内容场景下的快速定位与阅读辅助。组件支持激活项同步、悬浮展开 / 手动展开、搜索过滤、键盘导航、目标内容反馈等能力，并补充了组件导出、样式入口，以及少量 Bubble 渲染能力上的基础支持。
背景
在长文本、结构化内容或多段内容展示场景中，用户通常需要一个轻量、低接入成本的导航入口来：
因此，这次将内容导航能力抽离为独立组件
TrContentNav，并通过清晰的类型定义、内部 composables 和工具模块来组织实现。主要变更
1. 新增
content-nav组件模块新增
packages/components/src/content-nav/目录，主要包含以下内容：index.vuecomponents/ContentNavOverlay.vuecomponents/ContentNavList.vuecomponents/ContentNavItem.vuecomponents/ContentNavSearch.vueindex.type.tsinternal.type.tsdefaults.tsutils/scroll.tsutils/target.ts2. 将核心逻辑拆分为 composables
为降低主组件复杂度并提升可维护性，本次将核心行为拆分为多个 composables：
useNavControlleruseActiveSyncuseFloatingOffsetuseOverlayInteractionsuseTargetFeedback3. 实现内容导航的核心能力
当前
TrContentNav支持以下能力：left/right两种放置方向hover/manual两种展开触发模式activeId的受控 / 非受控使用方式expanded的受控 / 非受控使用方式query的受控 / 非受控使用方式item/marker/search/empty插槽扩展targetActiveClass/targetActiveDuration为目标内容提供跳转反馈tooltipDelay控制 tooltip 显示时机4. 增加内容目标关联机制
组件通过
data-content-nav-id与内容区域中的目标节点建立关联。这种方式保持了接入方式的轻量性，不要求业务额外包裹复杂结构，也便于后续在长文档、普通内容区、Bubble 等不同渲染场景中复用。
5. 新增
content-nav样式及变量新增
packages/components/src/styles/components/content-nav.less，并接入统一样式入口。样式层面主要包括：
6. 对外导出组件与类型
更新组件库导出入口和样式入口，使
ContentNav/TrContentNav及相关公开类型可以从组件库根入口导出。7. 补充 Bubble 侧最小共享支持
为了支持后续将内容导航能力接入 Bubble 场景，本次同步补充了 Bubble 侧一处较小但必要的基础能力：
BubbleBoxRendererMatch.attributes从仅支持静态对象，扩展为支持：useBubbleBoxRenderer增加了对动态 attributes 的解析逻辑这部分改动范围较小，但为后续按消息 / 内容动态生成属性提供了更灵活的基础。
对外 API
Props
itemsscrollContaineractiveIdexpandedqueryplacementexpandTriggersearchtooltipDelaytargetActiveClasstargetActiveDurationemptyTextEmits
update:activeIdupdate:expandedupdate:queryselectactivateSlots
itemmarkersearchemptySummary by CodeRabbit
Release Notes
New Features
Styling