feat(content-nav): add content navigation component#322
Open
SonyLeo wants to merge 7 commits intoopentiny:developfrom
Open
feat(content-nav): add content navigation component#322SonyLeo wants to merge 7 commits intoopentiny:developfrom
SonyLeo wants to merge 7 commits intoopentiny:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a full ContentNav system (components, composables, types, utils, styles, plugin registration) and updates bubble renderer types/logic to support dynamic attribute resolvers for renderer matches. Changes
Sequence DiagramsequenceDiagram
participant User
participant TrContentNav
participant NavController as useNavController
participant ActiveSync as useActiveSync
participant Floating as useFloatingOffset
participant Interactions as useOverlayInteractions
participant TargetFeedback as useTargetFeedback
User->>TrContentNav: input / scroll / key events
TrContentNav->>ActiveSync: sync() / scrollTo(id)
ActiveSync->>NavController: update activeId
TrContentNav->>NavController: setSearchQuery / setExpanded
TrContentNav->>Floating: sync()
User->>Interactions: keyboard / mouse events
Interactions->>TrContentNav: handleSelect(id)
TrContentNav->>TargetFeedback: activate(id)
TargetFeedback->>TrContentNav: feedback cleared (timeout)
TrContentNav->>User: emit select / update events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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@ba6b3d0 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@ba6b3d0 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@ba6b3d0 commit: ba6b3d0 |
![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
该问题已经解决，详见在线示例 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 27-33: The setActiveId function can repeatedly emit the same
update:activeId during scroll/resize; change setActiveId (in useActiveSync) to
compute the effective previous id (prefer options.activeId?.value if controlled
else localActiveId.value) and only assign localActiveId.value and call
options.onUpdateActiveId if the new value differs from that previous id; ensure
you still handle undefined/unset cases and keep the existing check for
options.activeId to respect controlled usage.
In `@packages/components/src/content-nav/defaults.ts`:
- Around line 42-52: The current logic can activate items without DOM targets
and marks the last item for non-scrollable content; change the early-return and
bottom-detection to use resolved anchors and avoid non-scrollable triggers:
don't return items[0]?.id when anchors is empty (return undefined/null instead),
compute isAtBottom only when viewport.scrollHeight > viewport.clientHeight
(e.g., isAtBottom = viewport.scrollHeight > viewport.clientHeight && ...), and
when choosing fallbacks or initial active id use anchors[0]?.id or
anchors[anchors.length-1]?.id (not items) so only entries with actual targets
are activated; update references in this block (anchors, items, viewport,
isAtBottom, threshold, activeId) accordingly.
In `@packages/components/src/content-nav/index.vue`:
- Around line 69-77: The scroll-driven activation currently only emits
'update:activeId' from useActiveSync's onUpdateActiveId handler; add an
emit('activate', value) alongside emit('update:activeId', value) in the
onUpdateActiveId callback used when constructing useActiveSync (symbol:
useActiveSync, onUpdateActiveId) and update the component's emit contract
(ContentNavEmits) to include the 'activate' event with the appropriate payload
signature so consumers can differentiate activation from selection.
🪄 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: 9c680630-3176-4391-a9ca-494c0414f598
📒 Files selected for processing (9)
packages/components/src/bubble/BubbleList.vuepackages/components/src/content-nav/components/ContentNavSearch.vuepackages/components/src/content-nav/composables/useActiveSync.tspackages/components/src/content-nav/composables/useNavController.tspackages/components/src/content-nav/composables/useTargetFeedback.tspackages/components/src/content-nav/defaults.tspackages/components/src/content-nav/index.type.tspackages/components/src/content-nav/index.vuepackages/components/src/content-nav/internal.type.ts
✅ Files skipped from review due to trivial changes (1)
- packages/components/src/content-nav/internal.type.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/src/content-nav/components/ContentNavSearch.vue
- packages/components/src/content-nav/composables/useNavController.ts
…avior in navigation
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 渲染能力上的基础支持。
背景
在长文本、结构化内容或多段内容展示场景中，用户通常需要一个轻量、低接入成本的导航入口来：
主要变更
1. 新增
content-nav组件模块新增
packages/components/src/content-nav/目录，主要包含以下内容：index.tsindex.vuecomponents/ContentNavOverlay.vuecomponents/ContentNavList.vuecomponents/ContentNavItem.vuecomponents/ContentNavSearch.vueindex.type.tsinternal.type.tsdefaults.tsutils/scroll.tsutils/target.ts2. 将核心逻辑拆分为 composables
为降低主组件复杂度并提升可维护性，本次将核心行为拆分为多个 composables：
useNavControlleruseActiveSyncupdate:activeIduseFloatingOffsetuseOverlayInteractionsuseTargetFeedback3. 实现内容导航的核心能力
当前
TrContentNav支持以下能力：left/right两种放置方向hover/manual两种展开触发模式activeId的受控 / 非受控使用方式searchQuery的受控 / 非受控使用方式searchOptions控制搜索区展示及搜索行为matcheractiveOffset调整激活项判定阈值item/marker/search/empty插槽扩展targetFeedbackClass/targetFeedbackDuration为目标内容提供跳转反馈tooltipDelay控制 tooltip 显示时机说明：
expanded仅在expandTrigger="manual"时作为外部控制状态生效；hover模式下由组件内部管理展开状态。4. 增加内容目标关联机制
组件通过
data-content-nav-id与内容区域中的目标节点建立关联。这种方式接入成本较低，不要求业务额外包裹复杂结构，适用于普通长内容区、长对话区以及 BubbleList 等不同渲染场景。
5. 新增
content-nav样式及变量新增
packages/components/src/styles/components/content-nav.less，并接入统一样式入口。样式层面主要包括：
6. 对外导出组件与类型
更新组件库导出入口和样式入口，使以下内容可从组件库根入口导出：
ContentNavTrContentNav7. 补充 Bubble 场景的共享支持
为了让
content-nav更自然地接入 Bubble 场景，本次同步补充了 Bubble 侧的基础能力：BubbleBoxRendererMatch.attributes从仅支持静态对象，扩展为支持：useBubbleBoxRenderer增加了对动态 attributes 的解析逻辑BubbleList通过defineExpose暴露rootElBubbleList自身作为scrollContainer使用对外 API
Props
itemsscrollContaineractiveIdactiveOffsetexpandedsearchQueryplacementexpandTriggersearchOptionstooltipDelaytargetFeedbackClasstargetFeedbackDurationemptyTextEmits
update:activeIdupdate:expandedupdate:searchQueryselectSlots
itemmarkersearchemptySummary by CodeRabbit
New Features
Style
Chores
Other