feat: add tooltip components - #45
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a Vue tooltip API, floating/portal infrastructure, positioning and transition utilities, demos, documentation, tests, public exports, a docs sidebar entry, and an ESLint rule override. ChangesTooltip component
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (12)
docs/components/demo/TooltipControlled/tailwind/index.vue (1)
18-21: ⚡ Quick winReplace
anyon tooltip event details with an explicit type.Line 18 uses
details: any, which weakens type safety in this TS/Vue demo and makesdetails.trigger?.idunchecked. Use a typed event-details shape (or the exportedTooltipRootevent-details type) instead.As per coding guidelines:
**/*.{ts,tsx,vue}: Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especiallyas any).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/components/demo/TooltipControlled/tailwind/index.vue` around lines 18 - 21, In the handleOpenChange function, replace the `details: any` parameter with an explicit, strongly-typed event-details type instead of using `any`. Either use the exported event-details type from TooltipRoot (if available) or define a custom typed shape that includes the trigger property with an optional id field. This will ensure type safety for the details.trigger?.id access and comply with the coding guideline to avoid unnecessary use of `any` in TypeScript/Vue files.Source: Coding guidelines
packages/core/src/utils/usePopupViewport.ts (1)
92-97: ⚡ Quick winStrengthen exported viewport prop typings (avoid
anyin public API).Lines 92 and 96 expose
ComputedRef<Record<string, any>>inUsePopupViewportResult, which weakens downstream type safety. Preferunknownor a concrete attribute/DOM props type.Suggested fix
- currentContainerProps: ComputedRef<Record<string, any>> + currentContainerProps: ComputedRef<Record<string, unknown>> @@ - previousContainerProps: ComputedRef<Record<string, any>> + previousContainerProps: ComputedRef<Record<string, unknown>>As per coding guidelines, "Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especially
as any)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/utils/usePopupViewport.ts` around lines 92 - 97, The UsePopupViewportResult interface exposes currentContainerProps and previousContainerProps with ComputedRef<Record<string, any>> types, which weakens type safety in the public API. Replace the any type in both ComputedRef<Record<string, any>> declarations with either unknown or a concrete type representing actual DOM/container attributes. This strengthens the exported API typing and improves downstream type safety for consumers of usePopupViewport.Source: Coding guidelines
packages/core/src/tooltip/trigger/TooltipTrigger.vue (1)
73-77: Tightenanyusage in trigger typing.
event?: any(line 75) andattrs as Record<string, any>(line 249) weaken type safety. Theeventparameter receivesMouseEvent | FocusEvent | KeyboardEvent | PointerEventbut acceptsany. Replace with the precise union orEvent | undefined. The cast on line 249 is redundant sinceuseAttrs()already returnsRecord<string, any>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/tooltip/trigger/TooltipTrigger.vue` around lines 73 - 77, Replace the overly broad `any` type for the `event` parameter in the `makeDetails` function with a precise union type `MouseEvent | FocusEvent | KeyboardEvent | PointerEvent | undefined` to reflect the actual event types it receives, and remove the redundant `as Record<string, any>` type cast where `useAttrs()` is used since `useAttrs()` already returns `Record<string, any>` without needing explicit casting.Source: Coding guidelines
packages/core/src/tooltip/trigger/TooltipTrigger.test.ts (1)
30-33: ⚡ Quick winReplace
anyforopen-changedetails in this test callback.
anyremoves static checks for the event-detail API used in the assertion path.Suggested update
+import type { TooltipRootChangeEventDetails } from '../root/TooltipRoot.vue' ... - function handleOpenChange(nextOpen: boolean, details: any) { + function handleOpenChange(nextOpen: boolean, details: TooltipRootChangeEventDetails) {As per coding guidelines, “Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especially
as any)”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/tooltip/trigger/TooltipTrigger.test.ts` around lines 30 - 33, The handleOpenChange callback function uses any as the type for the details parameter, which removes static type checking for the event-detail API. Replace the any type with the proper, strongly-typed interface that represents the open-change event details. This will ensure type safety when calling methods like preventUnmountOnClose() and allow the test to benefit from static analysis and IDE support for the event details API.Source: Coding guidelines
packages/core/src/tooltip/root/TooltipRoot.test.ts (1)
195-198: ⚡ Quick winType
open-changedetails explicitly in handler tests.Using
anyhere bypasses event-detail contract checks and can hide regressions in emitted payload shape.Suggested update
+import type { TooltipRootChangeEventDetails } from './TooltipRoot.vue' ... - function handleOpenChange(nextOpen: boolean, details: any) { + function handleOpenChange(nextOpen: boolean, details: TooltipRootChangeEventDetails) { ... - const handleOpenChange = vi.fn((nextOpen: boolean, details: any) => { + const handleOpenChange = vi.fn((nextOpen: boolean, details: TooltipRootChangeEventDetails) => {As per coding guidelines, “Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especially
as any)”.Also applies to: 262-265
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/tooltip/root/TooltipRoot.test.ts` around lines 195 - 198, The handleOpenChange function parameter details is typed as any, which bypasses type checking and can hide regressions in the emitted payload shape. Replace the any type with an explicit type that represents the open-change event details object. The type should reflect the structure being accessed (trigger property with id), ensuring proper type safety. Apply the same explicit typing fix to the similar handler at lines 262-265.Source: Coding guidelines
packages/core/src/tooltip/viewport/TooltipViewport.test.ts (1)
21-21: ⚡ Quick winRemove the
as anycast at Line 21 to keep the test strongly typed.Use a typed property delete instead of casting the prototype to
any.Suggested change
- delete (window.HTMLElement.prototype as any).getAnimations + Reflect.deleteProperty(window.HTMLElement.prototype, 'getAnimations')As per coding guidelines,
**/*.{ts,tsx,vue}: Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especiallyas any).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/tooltip/viewport/TooltipViewport.test.ts` at line 21, The delete statement at Line 21 in TooltipViewport.test.ts uses an unnecessary `as any` cast when deleting the getAnimations property from the HTMLElement prototype. Remove this cast by using bracket notation for property access instead, which provides type safety without resorting to `any`. Replace the current delete statement that casts the prototype to `any` with a version that accesses the property using string key notation, allowing TypeScript to properly type-check the deletion without losing type safety.Source: Coding guidelines
packages/core/src/floating-ui-vue/components/FloatingTree.ts (1)
46-58: ⚡ Quick winConsider adding JSDoc comments for consistency.
The exported hooks
useFloatingParentNodeId,useFloatingTree, anduseFloatingNodeIdhave JSDoc comments, butprovideFloatingNodeandprovideFloatingTreelack documentation. Adding JSDoc to these public APIs would improve consistency and developer experience.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/floating-ui-vue/components/FloatingTree.ts` around lines 46 - 58, Add JSDoc comments to the exported functions provideFloatingNode and provideFloatingTree to match the documentation style of other exported functions in the file. For provideFloatingNode, document that it provides a floating node context with the given id and parent node id. For provideFloatingTree, document that it provides a floating tree store context, accepts an optional external tree store, and returns the tree instance. Follow the same JSDoc format and structure used for the other exported hooks in this file to maintain consistency.packages/core/src/floating-ui-vue/utils/element.ts (1)
12-16: Remove unnecessaryas Elementcast after type guard.The
isElement()function from@floating-ui/utils/domis a proper TypeScript type guard (value is Element). After the guard on line 12, TypeScript automatically narrowstargettoElement, making the cast on line 16 unnecessary and violating the guideline to avoid unnecessary casts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/floating-ui-vue/utils/element.ts` around lines 12 - 16, The `isElement()` function is a proper TypeScript type guard that narrows the type of `target` to `Element` after the check on line 12. Remove the unnecessary `as Element` cast on line 16 where `targetElement` is assigned from `target`, since TypeScript has already narrowed the type and the cast is redundant. Simply assign `target` directly to `targetElement` without the explicit type assertion.Source: Coding guidelines
packages/core/src/floating-ui-vue/utils/createAttribute.ts (1)
1-3: ⚡ Quick winAdd explicit return type for API clarity.
The function lacks an explicit return type annotation. Adding
: stringwould make the public API contract more explicit.♻️ Proposed fix
-export function createAttribute(name: string) { +export function createAttribute(name: string): string { return `data-base-ui-vue-${name}` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/floating-ui-vue/utils/createAttribute.ts` around lines 1 - 3, The createAttribute function lacks an explicit return type annotation which reduces API clarity. Add an explicit return type annotation of `: string` after the function parameters and before the opening brace to make the return type contract explicit and improve type safety for callers of this exported function.packages/core/src/utils/popupStateMapping.ts (1)
78-80: ⚡ Quick winConsider using an enum for the
'data-instant'attribute.Line 79 hardcodes
'data-instant'as a string literal, while other attributes use values fromCommonPopupDataAttributesorCommonTriggerDataAttributesenums. This inconsistency could make the attribute harder to discover and maintain.♻️ Proposed improvement
If
data-instantis a common popup attribute, add it to the enum:export enum CommonPopupDataAttributes { // ... existing attributes align = 'data-align', + /** + * Indicates instant/immediate state. + */ + instant = 'data-instant', }Then use it in the mapping:
instant(value) { - return value ? { 'data-instant': value } : null + return value ? { [CommonPopupDataAttributes.instant]: value } : null },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/utils/popupStateMapping.ts` around lines 78 - 80, The instant function in popupStateMapping.ts hardcodes the string 'data-instant' instead of using an enum constant like other attributes do. To fix this inconsistency, add 'data-instant' as a constant to the CommonPopupDataAttributes enum (or the appropriate enum for common popup attributes if that exists elsewhere), then update the instant function to use that enum constant instead of the hardcoded string literal 'data-instant'. This improves maintainability and discoverability of the attribute.packages/core/src/floating-ui-vue/components/FloatingPortal.vue (1)
94-94: ⚡ Quick winAvoid
anyin forwarded attrs typing.
elementPropscurrently casts toRecord<string, any>, which weakens the public typing surface for portal props forwarding.♻️ Suggested change
- elementProps: computed(() => attrs as Record<string, any>), + elementProps: computed(() => attrs as Record<string, unknown>),As per coding guidelines,
**/*.{ts,tsx,vue}: Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especiallyas any).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/floating-ui-vue/components/FloatingPortal.vue` at line 94, The elementProps computed property in FloatingPortal.vue is using a cast to Record<string, any>, which weakens type safety. Remove the as any cast from the elementProps computed property definition and instead rely on the actual type of attrs or define an explicit, strongly-typed interface for the forwarded props. This maintains proper type information in the public API surface without resorting to any-type weakening.Source: Coding guidelines
packages/core/src/utils/FloatingPortalLite.vue (1)
32-32: ⚡ Quick winUse a non-
anyattrs type forelementProps.
Record<string, any>here weakens API typing and bypasses useful TypeScript checks.♻️ Suggested change
- elementProps: computed(() => attrs as Record<string, any>), + elementProps: computed(() => attrs as Record<string, unknown>),As per coding guidelines,
**/*.{ts,tsx,vue}: Prefer explicit, strongly typed public APIs; avoid unnecessary casts (especiallyas any).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/utils/FloatingPortalLite.vue` at line 32, Remove the explicit type cast `as Record<string, any>` from the attrs assignment in the elementProps computed property in FloatingPortalLite.vue. Instead of casting attrs to a weakly-typed Record, use the attrs directly or apply a more specific, strongly-typed interface that matches the actual expected prop types for this component. This maintains type safety and enables better TypeScript checking throughout the codebase.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/components/demo/Tooltip/tailwind/index.vue`:
- Line 13: The popupClass definitions in both files use the deprecated Tailwind
v3 bracket syntax for CSS variables which is incompatible with Tailwind v4. In
docs/components/demo/Tooltip/tailwind/index.vue at line 13, replace the Tailwind
utility `origin-[var(--transform-origin)]` with `origin-(--transform-origin)` in
the popupClass string. In
docs/components/demo/TooltipDetached/tailwind/index.vue at line 15, make the
same replacement of `origin-[var(--transform-origin)]` with
`origin-(--transform-origin)` in its popupClass string. Tailwind v4 requires
parentheses instead of brackets for arbitrary CSS variable references and does
not require the var() wrapper.
In `@docs/components/demo/TooltipMultiple/css/styles.css`:
- Around line 34-36: The Stylelint rule `declaration-empty-line-before` is
violated in the `.Viewport` selector because there is no blank line separating
the custom property `--viewport-inline-padding` from the regular property
`box-sizing`. Add a blank line after the `--viewport-inline-padding` declaration
and before the `box-sizing` declaration to separate the custom properties block
from the regular CSS properties block.
In `@docs/content/docs/components/tooltip.md`:
- Line 15: The documentation at line 15 currently requires an `aria-label`
attribute for every tooltip trigger, but this is overly restrictive since
triggers with visible text already have an accessible name. Modify the
requirement to state that the trigger must have an accessible name, which can be
provided either through visible text or an `aria-label` attribute when the
trigger lacks visible text. This aligns with accessibility best practices and
matches the documentation examples shown elsewhere that demonstrate text-labeled
triggers.
In `@packages/core/src/floating-ui-vue/components/FloatingPortal.ts`:
- Around line 81-91: The resolvePortalTarget function has redundant and
incorrect ref-unwrapping logic. The toValue call already unwraps refs, getters,
and plain values completely, so the subsequent isRef check on resolvedTarget
will never be true since toValue returns the unwrapped value, not a ref. Remove
the redundant ternary operator and simply return the resolvedTarget directly, as
it is already fully unwrapped at that point.
In `@packages/core/src/floating-ui-vue/hooks/useDismiss.ts`:
- Around line 261-274: The getOutsidePressEvent function does not handle the
case where the outsidePressEvent callback returns undefined. When resolved is
undefined (which is allowed by the callback type definition), attempting to
access resolved[computedType] on the final line will throw a runtime error. Add
a null coalescing or fallback check to provide a default value (such as
'sloppy') when the resolved value is undefined before attempting to access it
with the bracket notation.
In `@packages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.ts`:
- Around line 185-187: The tree.events.on('floating.closed', onNodeClosed) call
in the branch checking hasParentChildren() and tree is being re-attached on
every execution without removing the previous listener, causing duplicate
subscriptions where onNodeClosed fires multiple times on close events. Remove
the old listener before re-attaching by calling
tree.events.off('floating.closed', onNodeClosed) prior to the .on() call, or
implement a check to ensure the listener is only attached once during the
component lifecycle.
In `@packages/core/src/floating-ui-vue/middleware/arrow.ts`:
- Around line 62-69: The offsetParent setting is being computed into the
arrowOffsetParent variable but is never actually used to determine where
clientSize is retrieved from. The clientSize is always computed from
elements.floating regardless of whether arrowOffsetParent points to 'real' or
'floating'. Fix this by using the arrowOffsetParent variable to determine which
element to get the clientSize from: when offsetParent is 'real', retrieve
clientSize from arrowOffsetParent using the appropriate property accessor, and
only fall back to elements.floating if the primary calculation fails. Remove the
redundant duplicate assignment on the subsequent line.
In `@packages/core/src/floating-ui-vue/utils/constants.ts`:
- Line 6: The TYPEABLE_SELECTOR constant currently only matches elements with
contenteditable="true", but it excludes other valid editable contenteditable
states such as <div contenteditable> (without an explicit value) and
contenteditable="plaintext-only". Update the selector to match any element with
a contenteditable attribute that is not explicitly set to false, using the CSS
selector pattern [contenteditable]:not([contenteditable="false"]) in place of
the current [contenteditable="true"] portion, so that all valid editable
contenteditable states are properly captured.
In `@packages/core/src/floating-ui-vue/utils/tabbable.ts`:
- Around line 180-187: The appendMatchingElements function and related
restoration logic in enableFocusInside only handle HTMLElement instances due to
the isHTMLElement type guard, but disableFocusInside can disable both
HTMLElement and SVGElement by setting tabindex="-1". Update the type guard in
appendMatchingElements and the corresponding restoration logic at lines 289-301
to also check for and include SVGElement instances, ensuring that SVG controls
disabled by disableFocusInside are properly restored when enableFocusInside is
called.
In `@packages/core/src/tooltip/store/TooltipHandle.ts`:
- Around line 199-215: The function createTooltipChangeEventDetails uses a type
escape with event?: any parameter and an unnecessary cast with as
TooltipRootChangeEventDetails. Replace the event parameter type from any to
Event, which pragmatically covers all mapped reason types for tooltip events.
Remove the unnecessary cast at the end of the return statement since the return
type of createChangeEventDetails already matches the declared
TooltipRootChangeEventDetails type exactly.
In `@packages/core/src/utils/useAnchorPositioning.ts`:
- Around line 317-344: The shift middleware in the shiftMiddleware configuration
has inverted axis mappings that contradict Floating UI's semantics. According to
Floating UI, mainAxis controls movement along the reference edge (the
side/placement axis) and crossAxis controls movement perpendicular to it (the
alignment axis). Currently, mainAxis is set to check collisionAvoidanceAlign and
crossAxis is set to crossAxisShiftEnabled, but these are reversed. Fix this by
swapping the axis assignments: mainAxis should be based on
collisionAvoidanceSide to enable shifting when side collision avoidance is set
to 'shift', and crossAxis should be based on collisionAvoidanceAlign to enable
shifting when align collision avoidance is set to 'shift', while still
respecting the other conditions that influence cross-axis shifting.
In `@packages/core/src/utils/usePopupViewport.ts`:
- Around line 154-159: The onAnimationsFinished callback is being called without
an AbortSignal, allowing stale callbacks from previous transitions to execute
and incorrectly clear previousContentNode and previousContentDimensions when the
trigger changes rapidly. Implement cancellation by creating an AbortController
to manage the signal lifecycle, pass the abort signal to the
onAnimationsFinished call, and ensure that whenever a new animation transition
is triggered, any previous AbortController is aborted to prevent stale callbacks
from executing and corrupting the state for newer transitions.
---
Nitpick comments:
In `@docs/components/demo/TooltipControlled/tailwind/index.vue`:
- Around line 18-21: In the handleOpenChange function, replace the `details:
any` parameter with an explicit, strongly-typed event-details type instead of
using `any`. Either use the exported event-details type from TooltipRoot (if
available) or define a custom typed shape that includes the trigger property
with an optional id field. This will ensure type safety for the
details.trigger?.id access and comply with the coding guideline to avoid
unnecessary use of `any` in TypeScript/Vue files.
In `@packages/core/src/floating-ui-vue/components/FloatingPortal.vue`:
- Line 94: The elementProps computed property in FloatingPortal.vue is using a
cast to Record<string, any>, which weakens type safety. Remove the as any cast
from the elementProps computed property definition and instead rely on the
actual type of attrs or define an explicit, strongly-typed interface for the
forwarded props. This maintains proper type information in the public API
surface without resorting to any-type weakening.
In `@packages/core/src/floating-ui-vue/components/FloatingTree.ts`:
- Around line 46-58: Add JSDoc comments to the exported functions
provideFloatingNode and provideFloatingTree to match the documentation style of
other exported functions in the file. For provideFloatingNode, document that it
provides a floating node context with the given id and parent node id. For
provideFloatingTree, document that it provides a floating tree store context,
accepts an optional external tree store, and returns the tree instance. Follow
the same JSDoc format and structure used for the other exported hooks in this
file to maintain consistency.
In `@packages/core/src/floating-ui-vue/utils/createAttribute.ts`:
- Around line 1-3: The createAttribute function lacks an explicit return type
annotation which reduces API clarity. Add an explicit return type annotation of
`: string` after the function parameters and before the opening brace to make
the return type contract explicit and improve type safety for callers of this
exported function.
In `@packages/core/src/floating-ui-vue/utils/element.ts`:
- Around line 12-16: The `isElement()` function is a proper TypeScript type
guard that narrows the type of `target` to `Element` after the check on line 12.
Remove the unnecessary `as Element` cast on line 16 where `targetElement` is
assigned from `target`, since TypeScript has already narrowed the type and the
cast is redundant. Simply assign `target` directly to `targetElement` without
the explicit type assertion.
In `@packages/core/src/tooltip/root/TooltipRoot.test.ts`:
- Around line 195-198: The handleOpenChange function parameter details is typed
as any, which bypasses type checking and can hide regressions in the emitted
payload shape. Replace the any type with an explicit type that represents the
open-change event details object. The type should reflect the structure being
accessed (trigger property with id), ensuring proper type safety. Apply the same
explicit typing fix to the similar handler at lines 262-265.
In `@packages/core/src/tooltip/trigger/TooltipTrigger.test.ts`:
- Around line 30-33: The handleOpenChange callback function uses any as the type
for the details parameter, which removes static type checking for the
event-detail API. Replace the any type with the proper, strongly-typed interface
that represents the open-change event details. This will ensure type safety when
calling methods like preventUnmountOnClose() and allow the test to benefit from
static analysis and IDE support for the event details API.
In `@packages/core/src/tooltip/trigger/TooltipTrigger.vue`:
- Around line 73-77: Replace the overly broad `any` type for the `event`
parameter in the `makeDetails` function with a precise union type `MouseEvent |
FocusEvent | KeyboardEvent | PointerEvent | undefined` to reflect the actual
event types it receives, and remove the redundant `as Record<string, any>` type
cast where `useAttrs()` is used since `useAttrs()` already returns
`Record<string, any>` without needing explicit casting.
In `@packages/core/src/tooltip/viewport/TooltipViewport.test.ts`:
- Line 21: The delete statement at Line 21 in TooltipViewport.test.ts uses an
unnecessary `as any` cast when deleting the getAnimations property from the
HTMLElement prototype. Remove this cast by using bracket notation for property
access instead, which provides type safety without resorting to `any`. Replace
the current delete statement that casts the prototype to `any` with a version
that accesses the property using string key notation, allowing TypeScript to
properly type-check the deletion without losing type safety.
In `@packages/core/src/utils/FloatingPortalLite.vue`:
- Line 32: Remove the explicit type cast `as Record<string, any>` from the attrs
assignment in the elementProps computed property in FloatingPortalLite.vue.
Instead of casting attrs to a weakly-typed Record, use the attrs directly or
apply a more specific, strongly-typed interface that matches the actual expected
prop types for this component. This maintains type safety and enables better
TypeScript checking throughout the codebase.
In `@packages/core/src/utils/popupStateMapping.ts`:
- Around line 78-80: The instant function in popupStateMapping.ts hardcodes the
string 'data-instant' instead of using an enum constant like other attributes
do. To fix this inconsistency, add 'data-instant' as a constant to the
CommonPopupDataAttributes enum (or the appropriate enum for common popup
attributes if that exists elsewhere), then update the instant function to use
that enum constant instead of the hardcoded string literal 'data-instant'. This
improves maintainability and discoverability of the attribute.
In `@packages/core/src/utils/usePopupViewport.ts`:
- Around line 92-97: The UsePopupViewportResult interface exposes
currentContainerProps and previousContainerProps with ComputedRef<Record<string,
any>> types, which weakens type safety in the public API. Replace the any type
in both ComputedRef<Record<string, any>> declarations with either unknown or a
concrete type representing actual DOM/container attributes. This strengthens the
exported API typing and improves downstream type safety for consumers of
usePopupViewport.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53331f7a-e106-4caa-a3ab-1319b50d766e
📒 Files selected for processing (87)
docs/.vitepress/config.tsdocs/components/demo/Tooltip/css/index.vuedocs/components/demo/Tooltip/css/styles.cssdocs/components/demo/Tooltip/tailwind/index.vuedocs/components/demo/TooltipControlled/css/index.vuedocs/components/demo/TooltipControlled/css/styles.cssdocs/components/demo/TooltipControlled/tailwind/index.vuedocs/components/demo/TooltipDetached/css/index.vuedocs/components/demo/TooltipDetached/css/styles.cssdocs/components/demo/TooltipDetached/tailwind/index.vuedocs/components/demo/TooltipDetachedTriggersFull/css/index.vuedocs/components/demo/TooltipDetachedTriggersFull/css/styles.cssdocs/components/demo/TooltipDetachedTriggersFull/tailwind/index.vuedocs/components/demo/TooltipMultiple/css/index.vuedocs/components/demo/TooltipMultiple/css/styles.cssdocs/components/demo/TooltipMultiple/tailwind/index.vuedocs/content/docs/components/tooltip.mdeslint.config.mjspackages/core/src/floating-ui-vue/components/FloatingPortal.test.tspackages/core/src/floating-ui-vue/components/FloatingPortal.tspackages/core/src/floating-ui-vue/components/FloatingPortal.vuepackages/core/src/floating-ui-vue/components/FloatingRootStore.tspackages/core/src/floating-ui-vue/components/FloatingTree.tspackages/core/src/floating-ui-vue/components/FloatingTreeStore.tspackages/core/src/floating-ui-vue/hooks/useDismiss.test.tspackages/core/src/floating-ui-vue/hooks/useDismiss.tspackages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.tspackages/core/src/floating-ui-vue/hooks/useHoverInteractionSharedState.tspackages/core/src/floating-ui-vue/hooks/useHoverShared.tspackages/core/src/floating-ui-vue/index.tspackages/core/src/floating-ui-vue/middleware/arrow.tspackages/core/src/floating-ui-vue/types.tspackages/core/src/floating-ui-vue/utils/composite.tspackages/core/src/floating-ui-vue/utils/constants.tspackages/core/src/floating-ui-vue/utils/createAttribute.tspackages/core/src/floating-ui-vue/utils/createEventEmitter.tspackages/core/src/floating-ui-vue/utils/element.tspackages/core/src/floating-ui-vue/utils/event.tspackages/core/src/floating-ui-vue/utils/nodes.tspackages/core/src/floating-ui-vue/utils/tabbable.tspackages/core/src/index.tspackages/core/src/tooltip/arrow/TooltipArrow.test.tspackages/core/src/tooltip/arrow/TooltipArrow.vuepackages/core/src/tooltip/arrow/TooltipArrowDataAttributes.tspackages/core/src/tooltip/index.tspackages/core/src/tooltip/popup/TooltipPopup.test.tspackages/core/src/tooltip/popup/TooltipPopup.vuepackages/core/src/tooltip/popup/TooltipPopupDataAttributes.tspackages/core/src/tooltip/portal/TooltipPortal.test.tspackages/core/src/tooltip/portal/TooltipPortal.vuepackages/core/src/tooltip/portal/TooltipPortalContext.tspackages/core/src/tooltip/positioner/TooltipPositioner.test.tspackages/core/src/tooltip/positioner/TooltipPositioner.vuepackages/core/src/tooltip/positioner/TooltipPositionerContext.tspackages/core/src/tooltip/positioner/TooltipPositionerCssVars.tspackages/core/src/tooltip/positioner/TooltipPositionerDataAttributes.tspackages/core/src/tooltip/provider/TooltipProvider.test.tspackages/core/src/tooltip/provider/TooltipProvider.vuepackages/core/src/tooltip/provider/TooltipProviderContext.tspackages/core/src/tooltip/root/TooltipRoot.test.tspackages/core/src/tooltip/root/TooltipRoot.vuepackages/core/src/tooltip/root/TooltipRootContext.tspackages/core/src/tooltip/store/TooltipHandle.tspackages/core/src/tooltip/trigger/TooltipTrigger.test.tspackages/core/src/tooltip/trigger/TooltipTrigger.vuepackages/core/src/tooltip/trigger/TooltipTriggerDataAttributes.tspackages/core/src/tooltip/utils/constants.tspackages/core/src/tooltip/viewport/TooltipViewport.test.tspackages/core/src/tooltip/viewport/TooltipViewport.vuepackages/core/src/tooltip/viewport/TooltipViewportCssVars.tspackages/core/src/tooltip/viewport/TooltipViewportDataAttributes.tspackages/core/src/utils/FloatingPortalLite.vuepackages/core/src/utils/FocusGuard.vuepackages/core/src/utils/adaptiveOriginMiddleware.tspackages/core/src/utils/addEventListener.tspackages/core/src/utils/constants.tspackages/core/src/utils/getCssDimensions.tspackages/core/src/utils/getDisabledMountTransitionStyles.tspackages/core/src/utils/hideMiddleware.tspackages/core/src/utils/mergeCleanups.tspackages/core/src/utils/platform.tspackages/core/src/utils/popupStateMapping.tspackages/core/src/utils/useAnchorPositioning.tspackages/core/src/utils/usePopupAutoResize.tspackages/core/src/utils/usePopupViewport.tspackages/core/src/utils/usePositioner.tspackages/core/src/utils/useRenderElement.ts
Comment on lines
+81
to
+91
| function resolvePortalTarget( | ||
| target: MaybeRefOrGetter<FloatingPortalTarget> | undefined, | ||
| ) { | ||
| if (target === undefined) { | ||
| return undefined | ||
| } | ||
|
|
||
| const resolvedTarget = toValue(target) | ||
|
|
||
| return isRef(resolvedTarget) ? resolvedTarget.value : resolvedTarget | ||
| } |
There was a problem hiding this comment.
Fix incorrect ref unwrapping logic in resolvePortalTarget.
The function calls toValue(target) which already unwraps refs, getters, and plain values. The subsequent isRef(resolvedTarget) check will never be true because toValue returns the unwrapped value, not a ref.
🐛 Proposed fix
function resolvePortalTarget(
target: MaybeRefOrGetter<FloatingPortalTarget> | undefined,
) {
if (target === undefined) {
return undefined
}
-
- const resolvedTarget = toValue(target)
-
- return isRef(resolvedTarget) ? resolvedTarget.value : resolvedTarget
+ return toValue(target)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/floating-ui-vue/components/FloatingPortal.ts` around lines
81 - 91, The resolvePortalTarget function has redundant and incorrect
ref-unwrapping logic. The toValue call already unwraps refs, getters, and plain
values completely, so the subsequent isRef check on resolvedTarget will never be
true since toValue returns the unwrapped value, not a ref. Remove the redundant
ternary operator and simply return the resolvedTarget directly, as it is already
fully unwrapped at that point.
Comment on lines
+317
to
+344
| const shiftDisabled | ||
| = collisionAvoidanceAlign === 'none' && collisionAvoidanceSide !== 'shift' | ||
| const crossAxisShiftEnabled | ||
| = !shiftDisabled && (sticky || shiftCrossAxis || collisionAvoidanceSide === 'shift') | ||
|
|
||
| const flipMiddleware = collisionAvoidanceSide === 'none' | ||
| ? null | ||
| : flip({ | ||
| ...commonCollisionProps.value, | ||
| padding: expandCollisionPadding(collisionPadding.value, 1), | ||
| mainAxis: !shiftCrossAxis && collisionAvoidanceSide === 'flip', | ||
| crossAxis: collisionAvoidanceAlign === 'flip' ? 'alignment' : false, | ||
| fallbackAxisSideDirection: collisionAvoidanceFallbackAxisSide, | ||
| }) | ||
|
|
||
| const shiftMiddleware = shiftDisabled | ||
| ? null | ||
| : shift((data) => { | ||
| const html = ownerDocument(data.elements.floating)?.documentElement | ||
|
|
||
| return { | ||
| ...commonCollisionProps.value, | ||
| rootBoundary: shiftCrossAxis && html | ||
| ? { x: 0, y: 0, width: html.clientWidth, height: html.clientHeight } | ||
| : undefined, | ||
| mainAxis: collisionAvoidanceAlign !== 'none', | ||
| crossAxis: crossAxisShiftEnabled, | ||
| limiter: sticky || shiftCrossAxis |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/core/src/utils && wc -l useAnchorPositioning.tsRepository: vuepont/base-ui-vue
Length of output: 91
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | head -100Repository: vuepont/base-ui-vue
Length of output: 4253
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '200,350p'Repository: vuepont/base-ui-vue
Length of output: 7052
🏁 Script executed:
rg -A 5 -B 5 "collisionAvoidanceSide\|collisionAvoidanceAlign" packages/core/src/utils/useAnchorPositioning.ts | head -100Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "useAnchorPositioning\|collisionAvoidance" 2>/dev/null | head -5Repository: vuepont/base-ui-vue
Length of output: 238
🏁 Script executed:
rg "shiftCrossAxis|shift.*mainAxis|shift.*crossAxis" packages/core/src --type ts -B 3 -A 3Repository: vuepont/base-ui-vue
Length of output: 3977
🏁 Script executed:
rg -A 10 "const shiftMiddleware" packages/core/src/utils/useAnchorPositioning.tsRepository: vuepont/base-ui-vue
Length of output: 502
🏁 Script executed:
cat packages/core/src/tooltip/positioner/TooltipPositioner.test.ts | head -200Repository: vuepont/base-ui-vue
Length of output: 4994
🏁 Script executed:
rg -B 5 -A 10 "collisionAvoidance\|side.*shift\|align.*shift" packages/core/src/tooltip/positioner/TooltipPositioner.test.ts | head -150Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
web_search
Floating UI shift middleware mainAxis crossAxis documentationRepository: vuepont/base-ui-vue
Length of output: 161
🌐 Web query:
Floating UI shift middleware mainAxis crossAxis what axis
💡 Result:
In Floating UI, the shift middleware adjusts the position of a floating element to ensure it stays within the boundaries of a container, preventing it from overflowing [1][2]. The terms mainAxis and crossAxis define the directions along which this shifting occurs [1][3]. mainAxis The mainAxis (default: true) controls shifting along the axis that runs parallel to the edge of the reference element [1][4]. For top and bottom placements, this corresponds to the x-axis [1][2]. For left and right placements, this corresponds to the y-axis [1][2]. By enabling this, the floating element slides along the reference element to stay in view [4]. crossAxis The crossAxis (default: false) controls shifting along the axis perpendicular to the reference edge—the opposite of the mainAxis [1][4]. Enabling this is generally discouraged because it can cause the floating element to overlap the reference element, and is usually better handled by the flip middleware [1][3][2]. Important Note on Terminology The definition of mainAxis and crossAxis in the shift middleware is effectively the inverse of their definition in the offset middleware [4]. While offset uses mainAxis to control the distance (gap) from the reference element [5], shift uses mainAxis to control the sliding motion along the reference element [4]. For example, in a bottom placement, offset views the Y-axis as the mainAxis (distance from reference), whereas shift views the X-axis as the mainAxis (sliding along the reference) [4].
Citations:
- 1: https://floating-ui.com/docs/shift
- 2: https://floating-ui.rustforweb.org/middleware/shift.html
- 3: https://folz.github.io/anchors/docs/shift
- 4: https://www.jinghuangsu.com/til/floating-ui-middleware-in-deep
- 5: https://floating-ui.com/docs/offset
🏁 Script executed:
rg -B 2 -A 15 "SideShiftMode\|SideFlipMode" packages/core/src/utils/useAnchorPositioning.tsRepository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '45,90p'Repository: vuepont/base-ui-vue
Length of output: 2331
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '280,290p'Repository: vuepont/base-ui-vue
Length of output: 470
🏁 Script executed:
rg -B 3 -A 8 "const placement" packages/core/src/utils/useAnchorPositioning.tsRepository: vuepont/base-ui-vue
Length of output: 425
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '322,330p'Repository: vuepont/base-ui-vue
Length of output: 563
🏁 Script executed:
rg "flip\|shift" packages/core/src/floating-ui-vue/middleware -l 2>/dev/null | head -5Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
find . -path "*/floating-ui*" -name "*.ts" -type f | grep -E "shift|flip" | head -10Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '313,350p'Repository: vuepont/base-ui-vue
Length of output: 1941
🏁 Script executed:
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | xargs rg "shiftMiddleware\|shift.*mainAxis\|shift.*crossAxis" 2>/dev/null | head -20Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '1,50p'Repository: vuepont/base-ui-vue
Length of output: 1908
🏁 Script executed:
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) -path "*/packages/core/src/*" | xargs rg -l "collisionAvoidance.*side.*shift\|collisionAvoidance.*align.*shift" 2>/dev/nullRepository: vuepont/base-ui-vue
Length of output: 47
🏁 Script executed:
rg "collisionAvoidance" packages/core/src -A 3 | grep -E "side.*shift|align.*shift" -B 2 -A 2Repository: vuepont/base-ui-vue
Length of output: 45
🌐 Web query:
Floating UI flip middleware mainAxis crossAxis placement axis semantics
💡 Result:
In Floating UI, the flip middleware changes the floating element's placement to keep it in view [1][2]. It uses two primary axes to determine when a flip should occur: the main axis and the cross axis [1][2]. Main Axis The main axis is the primary direction along which the floating element is positioned relative to the reference element [1]. - For 'top' and 'bottom' placements, the main axis is the y-axis [1]. - For 'left' and 'right' placements, the main axis is the x-axis [1]. By default, the flip middleware checks for overflow along this axis to determine if a flip is necessary [1][2]. Disabling this (mainAxis: false) causes the middleware to ignore overflow on this axis [1]. Cross Axis The cross axis is the axis perpendicular to the main axis [1]. It controls flipping related to the alignment of the floating element [1][3]. - It can be a boolean (default true) to enable or disable overflow checking on this axis [1][2]. - It also accepts the string 'alignment', which restricts cross-axis overflow checking specifically to alignment-based flipping [3]. Axis Semantics and Behavior The interaction between these axes defines the flip behavior [1]. When flip is used alongside other middleware like shift, they must be ordered carefully to manage placement states [4][5]. For example, shifting the placement logic to avoid stale state is a common requirement when combining these tools [4][5]. The 'alignment' option for crossAxis was introduced to provide more granular control, allowing the middleware to prioritize flipping on the main axis while still respecting alignment constraints [3][6][7].
Citations:
- 1: https://floating-ui.com/docs/flip
- 2: https://vfloat.pages.dev/api/flip
- 3: floating-ui/floating-ui@c81f259
- 4: https://floating-ui.com/docs/middleware
- 5: Combine flip and shift middleware doesn't keep original placement floating-ui/floating-ui#2766
- 6: floating-ui/floating-ui@37eb937
- 7: floating-ui/floating-ui@693c9cc
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '307,360p'Repository: vuepont/base-ui-vue
Length of output: 2952
🏁 Script executed:
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) -path "*/packages/core/src/*" | head -10Repository: vuepont/base-ui-vue
Length of output: 622
🏁 Script executed:
rg "getPhysicalSide\|getSide\|getSideAxis" packages/core/src/utils/useAnchorPositioning.ts -B 2 -A 5Repository: vuepont/base-ui-vue
Length of output: 45
🏁 Script executed:
cat -n packages/core/src/utils/useAnchorPositioning.ts | sed -n '69,82p'Repository: vuepont/base-ui-vue
Length of output: 671
🏁 Script executed:
python3 << 'PY'
# Verify axis semantics for shift middleware
# For a bottom placement:
# - Side axis: vertical (top/bottom placement choice)
# - Align axis: horizontal (start/center/end alignment)
# Floating UI shift middleware:
# - mainAxis: parallel to reference edge = horizontal (x-axis for bottom)
# - crossAxis: perpendicular = vertical (y-axis for bottom)
# API contracts:
# - collisionAvoidanceSide='shift': keep bottom, shift horizontally to fit
# - collisionAvoidanceAlign='shift': keep alignment, shift vertically to fit
# Expected mapping for shift middleware:
# - mainAxis (x-axis horizontal) should enable when collisionAvoidanceSide='shift'
# - crossAxis (y-axis vertical) should enable when collisionAvoidanceAlign='shift'
# Current code:
# - mainAxis: collisionAvoidanceAlign !== 'none' (BACKWARDS - uses align)
# - crossAxis: includes collisionAvoidanceSide === 'shift' (BACKWARDS - uses side)
print("Expected mapping:")
print(" mainAxis: collisionAvoidanceSide === 'shift'")
print(" crossAxis: collisionAvoidanceAlign === 'shift'")
print()
print("Current mapping:")
print(" mainAxis: collisionAvoidanceAlign !== 'none'")
print(" crossAxis: crossAxisShiftEnabled (includes collisionAvoidanceSide === 'shift')")
print()
print("VERDICT: Axes are INVERTED - review comment is CORRECT")
PYRepository: vuepont/base-ui-vue
Length of output: 379
Collision-avoidance shift middleware axes are inverted.
The shift middleware configuration maps collisionAvoidanceAlign to mainAxis and collisionAvoidanceSide to crossAxis, which contradicts Floating UI's shift semantics where mainAxis controls movement along the reference edge (the placement/side axis) and crossAxis controls movement perpendicular to the reference edge (the alignment axis). The API contract defines side: 'shift' as "keep the preferred side and shift to fit" (horizontal movement for bottom), and align: 'shift' as "keep the alignment and shift to fit" (vertical movement for bottom).
🐛 Suggested fix
- const crossAxisShiftEnabled
- = !shiftDisabled && (sticky || shiftCrossAxis || collisionAvoidanceSide === 'shift')
+ const crossAxisShiftEnabled
+ = !shiftDisabled && (sticky || shiftCrossAxis || collisionAvoidanceAlign === 'shift')
@@
- mainAxis: collisionAvoidanceAlign !== 'none',
+ mainAxis: collisionAvoidanceSide === 'shift' || sticky,
crossAxis: crossAxisShiftEnabled,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const shiftDisabled | |
| = collisionAvoidanceAlign === 'none' && collisionAvoidanceSide !== 'shift' | |
| const crossAxisShiftEnabled | |
| = !shiftDisabled && (sticky || shiftCrossAxis || collisionAvoidanceSide === 'shift') | |
| const flipMiddleware = collisionAvoidanceSide === 'none' | |
| ? null | |
| : flip({ | |
| ...commonCollisionProps.value, | |
| padding: expandCollisionPadding(collisionPadding.value, 1), | |
| mainAxis: !shiftCrossAxis && collisionAvoidanceSide === 'flip', | |
| crossAxis: collisionAvoidanceAlign === 'flip' ? 'alignment' : false, | |
| fallbackAxisSideDirection: collisionAvoidanceFallbackAxisSide, | |
| }) | |
| const shiftMiddleware = shiftDisabled | |
| ? null | |
| : shift((data) => { | |
| const html = ownerDocument(data.elements.floating)?.documentElement | |
| return { | |
| ...commonCollisionProps.value, | |
| rootBoundary: shiftCrossAxis && html | |
| ? { x: 0, y: 0, width: html.clientWidth, height: html.clientHeight } | |
| : undefined, | |
| mainAxis: collisionAvoidanceAlign !== 'none', | |
| crossAxis: crossAxisShiftEnabled, | |
| limiter: sticky || shiftCrossAxis | |
| const shiftDisabled | |
| = collisionAvoidanceAlign === 'none' && collisionAvoidanceSide !== 'shift' | |
| const crossAxisShiftEnabled | |
| = !shiftDisabled && (sticky || shiftCrossAxis || collisionAvoidanceAlign === 'shift') | |
| const flipMiddleware = collisionAvoidanceSide === 'none' | |
| ? null | |
| : flip({ | |
| ...commonCollisionProps.value, | |
| padding: expandCollisionPadding(collisionPadding.value, 1), | |
| mainAxis: !shiftCrossAxis && collisionAvoidanceSide === 'flip', | |
| crossAxis: collisionAvoidanceAlign === 'flip' ? 'alignment' : false, | |
| fallbackAxisSideDirection: collisionAvoidanceFallbackAxisSide, | |
| }) | |
| const shiftMiddleware = shiftDisabled | |
| ? null | |
| : shift((data) => { | |
| const html = ownerDocument(data.elements.floating)?.documentElement | |
| return { | |
| ...commonCollisionProps.value, | |
| rootBoundary: shiftCrossAxis && html | |
| ? { x: 0, y: 0, width: html.clientWidth, height: html.clientHeight } | |
| : undefined, | |
| mainAxis: collisionAvoidanceSide === 'shift' || sticky, | |
| crossAxis: crossAxisShiftEnabled, | |
| limiter: sticky || shiftCrossAxis |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/utils/useAnchorPositioning.ts` around lines 317 - 344, The
shift middleware in the shiftMiddleware configuration has inverted axis mappings
that contradict Floating UI's semantics. According to Floating UI, mainAxis
controls movement along the reference edge (the side/placement axis) and
crossAxis controls movement perpendicular to it (the alignment axis). Currently,
mainAxis is set to check collisionAvoidanceAlign and crossAxis is set to
crossAxisShiftEnabled, but these are reversed. Fix this by swapping the axis
assignments: mainAxis should be based on collisionAvoidanceSide to enable
shifting when side collision avoidance is set to 'shift', and crossAxis should
be based on collisionAvoidanceAlign to enable shifting when align collision
avoidance is set to 'shift', while still respecting the other conditions that
influence cross-axis shifting.
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/floating-ui-vue/hooks/useDismiss.ts`:
- Around line 269-277: Add a focused test case for the useDismiss hook that
verifies when the outsidePressEvent resolver returns null or undefined, the hook
falls back to returning 'sloppy' instead of throwing an error. This test should
lock down the nullish fallback contract to prevent future regressions in
event-resolution logic changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4de7a02b-0c7d-4c24-8830-3e9c2663ff00
📒 Files selected for processing (25)
docs/components/demo/Tooltip/tailwind/index.vuedocs/components/demo/TooltipControlled/css/index.vuedocs/components/demo/TooltipControlled/tailwind/index.vuedocs/components/demo/TooltipDetached/tailwind/index.vuedocs/components/demo/TooltipDetachedTriggersFull/tailwind/index.vuedocs/components/demo/TooltipMultiple/css/styles.cssdocs/components/demo/TooltipMultiple/tailwind/index.vuedocs/content/docs/components/tooltip.mdpackages/core/src/floating-ui-vue/components/FloatingPortal.vuepackages/core/src/floating-ui-vue/components/FloatingTree.tspackages/core/src/floating-ui-vue/hooks/useDismiss.tspackages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.tspackages/core/src/floating-ui-vue/middleware/arrow.tspackages/core/src/floating-ui-vue/utils/constants.tspackages/core/src/floating-ui-vue/utils/createAttribute.tspackages/core/src/floating-ui-vue/utils/element.tspackages/core/src/floating-ui-vue/utils/tabbable.tspackages/core/src/tooltip/root/TooltipRoot.test.tspackages/core/src/tooltip/store/TooltipHandle.tspackages/core/src/tooltip/trigger/TooltipTrigger.test.tspackages/core/src/tooltip/trigger/TooltipTrigger.vuepackages/core/src/tooltip/viewport/TooltipViewport.test.tspackages/core/src/utils/FloatingPortalLite.vuepackages/core/src/utils/popupStateMapping.tspackages/core/src/utils/usePopupViewport.ts
✅ Files skipped from review due to trivial changes (1)
- docs/content/docs/components/tooltip.md
🚧 Files skipped from review as they are similar to previous changes (21)
- packages/core/src/floating-ui-vue/utils/constants.ts
- docs/components/demo/Tooltip/tailwind/index.vue
- packages/core/src/floating-ui-vue/utils/createAttribute.ts
- packages/core/src/floating-ui-vue/components/FloatingTree.ts
- docs/components/demo/TooltipDetached/tailwind/index.vue
- docs/components/demo/TooltipMultiple/tailwind/index.vue
- packages/core/src/floating-ui-vue/middleware/arrow.ts
- docs/components/demo/TooltipDetachedTriggersFull/tailwind/index.vue
- packages/core/src/utils/FloatingPortalLite.vue
- docs/components/demo/TooltipControlled/css/index.vue
- docs/components/demo/TooltipControlled/tailwind/index.vue
- docs/components/demo/TooltipMultiple/css/styles.css
- packages/core/src/utils/usePopupViewport.ts
- packages/core/src/tooltip/viewport/TooltipViewport.test.ts
- packages/core/src/utils/popupStateMapping.ts
- packages/core/src/tooltip/trigger/TooltipTrigger.vue
- packages/core/src/tooltip/store/TooltipHandle.ts
- packages/core/src/tooltip/root/TooltipRoot.test.ts
- packages/core/src/floating-ui-vue/components/FloatingPortal.vue
- packages/core/src/floating-ui-vue/utils/tabbable.ts
- packages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.ts
Comment on lines
+269
to
+277
| if (resolved == null) { | ||
| return 'sloppy' | ||
| } | ||
|
|
||
| if (typeof resolved === 'string') { | ||
| return resolved | ||
| } | ||
|
|
||
| return resolved[computedType] ?? 'sloppy' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add regression coverage for the nullish outsidePressEvent fallback.
This branch changes dismissal behavior when the resolver returns null/undefined, but this review slice does not add a test that locks that contract down. Please add a focused case asserting the hook falls back to 'sloppy' instead of throwing, so future event-resolution changes do not silently regress it.
As per coding guidelines, "packages/core/src/**/*.{ts,tsx,vue}: Any behavior change in packages/core/src/ should include updated or new tests covering interaction/accessibility expectations."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/floating-ui-vue/hooks/useDismiss.ts` around lines 269 -
277, Add a focused test case for the useDismiss hook that verifies when the
outsidePressEvent resolver returns null or undefined, the hook falls back to
returning 'sloppy' instead of throwing an error. This test should lock down the
nullish fallback contract to prevent future regressions in event-resolution
logic changes.
Source: Coding guidelines
peoray
force-pushed
the
feature/tooltip-components
branch
from
June 15, 2026 00:11
e7c8e54 to
55f528a
Compare
Member
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/floating-ui-vue/components/FloatingRootStore.ts`:
- Around line 53-55: The select method in FloatingRootStore.ts contains an
unnecessary cast to FloatingRootState[typeof key]. Remove this cast and instead
add an explicit generic type parameter <K extends keyof FloatingRootState> to
the select method signature to match the public API contract defined in
types.ts. Change the method signature from select(key) to select<K extends keyof
FloatingRootState>(key: K) and remove the as FloatingRootState[typeof key] cast,
allowing TypeScript to infer the return type correctly and eliminating the need
for the cast.
In `@packages/core/src/tooltip/popup/TooltipPopup.vue`:
- Line 78: Remove the `as Record<string, any>` type cast from the attrs variable
in the TooltipPopup.vue file. The attrs parameter obtained from useAttrs()
already returns Record<string, unknown> in Vue 3.5+, which provides sufficient
type safety. Pass attrs directly to mergeProps without any explicit type casting
to maintain strong typing and align with the pattern used in similar components
like SwitchThumb.vue and SwitchRoot.vue.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eef51b5-4caf-432a-b6ea-4f8da1cfffa8
📒 Files selected for processing (86)
docs/.vitepress/config.tsdocs/components/demo/Tooltip/css/index.vuedocs/components/demo/Tooltip/css/styles.cssdocs/components/demo/Tooltip/tailwind/index.vuedocs/components/demo/TooltipControlled/css/index.vuedocs/components/demo/TooltipControlled/css/styles.cssdocs/components/demo/TooltipControlled/tailwind/index.vuedocs/components/demo/TooltipDetached/css/index.vuedocs/components/demo/TooltipDetached/css/styles.cssdocs/components/demo/TooltipDetached/tailwind/index.vuedocs/components/demo/TooltipDetachedTriggersFull/css/index.vuedocs/components/demo/TooltipDetachedTriggersFull/css/styles.cssdocs/components/demo/TooltipDetachedTriggersFull/tailwind/index.vuedocs/components/demo/TooltipMultiple/css/index.vuedocs/components/demo/TooltipMultiple/css/styles.cssdocs/components/demo/TooltipMultiple/tailwind/index.vuedocs/content/docs/components/tooltip.mdeslint.config.mjspackages/core/src/floating-ui-vue/components/FloatingPortal.test.tspackages/core/src/floating-ui-vue/components/FloatingPortal.tspackages/core/src/floating-ui-vue/components/FloatingPortal.vuepackages/core/src/floating-ui-vue/components/FloatingRootStore.tspackages/core/src/floating-ui-vue/components/FloatingTree.tspackages/core/src/floating-ui-vue/components/FloatingTreeStore.tspackages/core/src/floating-ui-vue/hooks/useDismiss.test.tspackages/core/src/floating-ui-vue/hooks/useDismiss.tspackages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.tspackages/core/src/floating-ui-vue/hooks/useHoverInteractionSharedState.tspackages/core/src/floating-ui-vue/hooks/useHoverShared.tspackages/core/src/floating-ui-vue/index.tspackages/core/src/floating-ui-vue/middleware/arrow.tspackages/core/src/floating-ui-vue/types.tspackages/core/src/floating-ui-vue/utils/composite.tspackages/core/src/floating-ui-vue/utils/constants.tspackages/core/src/floating-ui-vue/utils/createAttribute.tspackages/core/src/floating-ui-vue/utils/createEventEmitter.tspackages/core/src/floating-ui-vue/utils/element.tspackages/core/src/floating-ui-vue/utils/event.tspackages/core/src/floating-ui-vue/utils/nodes.tspackages/core/src/floating-ui-vue/utils/tabbable.tspackages/core/src/index.tspackages/core/src/tooltip/arrow/TooltipArrow.test.tspackages/core/src/tooltip/arrow/TooltipArrow.vuepackages/core/src/tooltip/arrow/TooltipArrowDataAttributes.tspackages/core/src/tooltip/index.tspackages/core/src/tooltip/popup/TooltipPopup.test.tspackages/core/src/tooltip/popup/TooltipPopup.vuepackages/core/src/tooltip/popup/TooltipPopupDataAttributes.tspackages/core/src/tooltip/portal/TooltipPortal.test.tspackages/core/src/tooltip/portal/TooltipPortal.vuepackages/core/src/tooltip/portal/TooltipPortalContext.tspackages/core/src/tooltip/positioner/TooltipPositioner.test.tspackages/core/src/tooltip/positioner/TooltipPositioner.vuepackages/core/src/tooltip/positioner/TooltipPositionerContext.tspackages/core/src/tooltip/positioner/TooltipPositionerCssVars.tspackages/core/src/tooltip/positioner/TooltipPositionerDataAttributes.tspackages/core/src/tooltip/provider/TooltipProvider.test.tspackages/core/src/tooltip/provider/TooltipProvider.vuepackages/core/src/tooltip/provider/TooltipProviderContext.tspackages/core/src/tooltip/root/TooltipRoot.test.tspackages/core/src/tooltip/root/TooltipRoot.vuepackages/core/src/tooltip/root/TooltipRootContext.tspackages/core/src/tooltip/store/TooltipHandle.tspackages/core/src/tooltip/trigger/TooltipTrigger.test.tspackages/core/src/tooltip/trigger/TooltipTrigger.vuepackages/core/src/tooltip/trigger/TooltipTriggerDataAttributes.tspackages/core/src/tooltip/utils/constants.tspackages/core/src/tooltip/viewport/TooltipViewport.test.tspackages/core/src/tooltip/viewport/TooltipViewport.vuepackages/core/src/tooltip/viewport/TooltipViewportCssVars.tspackages/core/src/tooltip/viewport/TooltipViewportDataAttributes.tspackages/core/src/utils/FloatingPortalLite.vuepackages/core/src/utils/FocusGuard.vuepackages/core/src/utils/adaptiveOriginMiddleware.tspackages/core/src/utils/addEventListener.tspackages/core/src/utils/constants.tspackages/core/src/utils/getDisabledMountTransitionStyles.tspackages/core/src/utils/hideMiddleware.tspackages/core/src/utils/mergeCleanups.tspackages/core/src/utils/platform.tspackages/core/src/utils/popupStateMapping.tspackages/core/src/utils/useAnchorPositioning.tspackages/core/src/utils/usePopupAutoResize.tspackages/core/src/utils/usePopupViewport.tspackages/core/src/utils/usePositioner.tspackages/core/src/utils/useRenderElement.ts
💤 Files with no reviewable changes (3)
- packages/core/src/utils/useRenderElement.ts
- packages/core/src/utils/usePositioner.ts
- packages/core/src/utils/usePopupViewport.ts
✅ Files skipped from review due to trivial changes (5)
- packages/core/src/tooltip/viewport/TooltipViewportCssVars.ts
- packages/core/src/tooltip/utils/constants.ts
- packages/core/src/floating-ui-vue/utils/createAttribute.ts
- packages/core/src/tooltip/positioner/TooltipPositionerCssVars.ts
- packages/core/src/floating-ui-vue/types.ts
🚧 Files skipped from review as they are similar to previous changes (74)
- docs/.vitepress/config.ts
- packages/core/src/tooltip/viewport/TooltipViewportDataAttributes.ts
- docs/components/demo/Tooltip/tailwind/index.vue
- packages/core/src/floating-ui-vue/utils/constants.ts
- packages/core/src/utils/hideMiddleware.ts
- packages/core/src/tooltip/popup/TooltipPopupDataAttributes.ts
- packages/core/src/utils/getDisabledMountTransitionStyles.ts
- packages/core/src/tooltip/trigger/TooltipTriggerDataAttributes.ts
- packages/core/src/utils/mergeCleanups.ts
- docs/components/demo/Tooltip/css/index.vue
- packages/core/src/floating-ui-vue/components/FloatingTreeStore.ts
- docs/components/demo/TooltipDetached/tailwind/index.vue
- eslint.config.mjs
- packages/core/src/floating-ui-vue/utils/createEventEmitter.ts
- packages/core/src/utils/FloatingPortalLite.vue
- docs/components/demo/TooltipControlled/css/styles.css
- packages/core/src/tooltip/positioner/TooltipPositionerDataAttributes.ts
- packages/core/src/utils/platform.ts
- docs/components/demo/Tooltip/css/styles.css
- packages/core/src/tooltip/arrow/TooltipArrowDataAttributes.ts
- packages/core/src/floating-ui-vue/utils/nodes.ts
- packages/core/src/tooltip/positioner/TooltipPositionerContext.ts
- packages/core/src/tooltip/provider/TooltipProviderContext.ts
- packages/core/src/tooltip/viewport/TooltipViewport.test.ts
- packages/core/src/tooltip/root/TooltipRootContext.ts
- docs/components/demo/TooltipMultiple/css/styles.css
- packages/core/src/utils/constants.ts
- packages/core/src/tooltip/viewport/TooltipViewport.vue
- docs/components/demo/TooltipMultiple/css/index.vue
- packages/core/src/utils/FocusGuard.vue
- packages/core/src/index.ts
- docs/components/demo/TooltipDetached/css/index.vue
- docs/components/demo/TooltipMultiple/tailwind/index.vue
- packages/core/src/floating-ui-vue/components/FloatingPortal.ts
- packages/core/src/floating-ui-vue/hooks/useHoverShared.ts
- packages/core/src/tooltip/trigger/TooltipTrigger.test.ts
- packages/core/src/utils/adaptiveOriginMiddleware.ts
- packages/core/src/floating-ui-vue/utils/composite.ts
- docs/components/demo/TooltipControlled/css/index.vue
- packages/core/src/tooltip/arrow/TooltipArrow.test.ts
- packages/core/src/floating-ui-vue/index.ts
- packages/core/src/tooltip/popup/TooltipPopup.test.ts
- docs/components/demo/TooltipControlled/tailwind/index.vue
- packages/core/src/utils/addEventListener.ts
- packages/core/src/floating-ui-vue/hooks/useHoverInteractionSharedState.ts
- packages/core/src/tooltip/arrow/TooltipArrow.vue
- packages/core/src/tooltip/index.ts
- packages/core/src/floating-ui-vue/middleware/arrow.ts
- packages/core/src/tooltip/portal/TooltipPortalContext.ts
- docs/components/demo/TooltipDetached/css/styles.css
- docs/components/demo/TooltipDetachedTriggersFull/tailwind/index.vue
- packages/core/src/floating-ui-vue/utils/event.ts
- packages/core/src/tooltip/provider/TooltipProvider.vue
- packages/core/src/floating-ui-vue/utils/element.ts
- packages/core/src/floating-ui-vue/components/FloatingPortal.test.ts
- packages/core/src/tooltip/root/TooltipRoot.vue
- packages/core/src/tooltip/portal/TooltipPortal.vue
- packages/core/src/tooltip/store/TooltipHandle.ts
- packages/core/src/floating-ui-vue/hooks/useDismiss.test.ts
- packages/core/src/tooltip/provider/TooltipProvider.test.ts
- packages/core/src/tooltip/portal/TooltipPortal.test.ts
- packages/core/src/tooltip/root/TooltipRoot.test.ts
- packages/core/src/floating-ui-vue/hooks/useHoverFloatingInteraction.ts
- packages/core/src/utils/useAnchorPositioning.ts
- packages/core/src/tooltip/trigger/TooltipTrigger.vue
- packages/core/src/tooltip/positioner/TooltipPositioner.test.ts
- packages/core/src/floating-ui-vue/components/FloatingPortal.vue
- docs/components/demo/TooltipDetachedTriggersFull/css/index.vue
- packages/core/src/utils/usePopupAutoResize.ts
- packages/core/src/tooltip/positioner/TooltipPositioner.vue
- packages/core/src/floating-ui-vue/components/FloatingTree.ts
- packages/core/src/floating-ui-vue/utils/tabbable.ts
- packages/core/src/floating-ui-vue/hooks/useDismiss.ts
- packages/core/src/utils/popupStateMapping.ts
…ish resolver in useDismiss
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
1 participant
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.
Summary
Describe the change and why it is needed.
Linked issue
Type of change
Description
Testing
pnpm lintpnpm --filter base-ui-vue type-checkpnpm --filter base-ui-vue testpnpm docs:build(if docs changed)Screenshots
Add screenshots, recordings, or visual diffs if they help explain the change.
Checklist
packages/core/src/.Summary by CodeRabbit