feat: support Shadow DOM teleport compatibility#124
feat: support Shadow DOM teleport compatibility#124hexqi merged 9 commits intoopentiny:release/v0.2.xfrom
Conversation
- Added `onItemClick`, `onClickOutside`, `onOpen`, and `onClose` event props to `SuggestionPopover` and `DropdownMenu` for improved flexibility. - Updated `SuggestionPill` actions to utilize new event props, replacing deprecated event handling. - Refactored `PillButtonWrapper` to support new event props for better integration with suggestion actions. - Improved handling of click events and visibility toggling in `SuggestionPills` and related components.
…port - Introduced a `trigger` slot in `DropdownMenu` and `PillButtonWrapper` components to allow custom trigger elements. - Refactored event handling for dropdown menus to utilize a centralized `closeAllDropdownMenus` function for improved code clarity. - Enhanced the `DropdownMenu` component to support teleporting the dropdown menu to the trigger element's position. - Updated related demo files to reflect the new slot structure for better usability.
…components - Changed item IDs in `pills-popper-config.vue` to start from 2 for better uniqueness. - Refactored visibility logic in `index.vue` to simplify conditions for showing static and floating items based on button visibility and state. - Removed unnecessary gap calculation function and replaced it with a fixed gap value for consistency.
…ity handling - Introduced a button in `pills-popper-config.vue` to allow users to dynamically add items to the suggestion list. - Implemented a `closeAllPopper` function to streamline the visibility management of suggestion items. - Enhanced click handling to ensure proper visibility toggling when interacting with the new button.
… into component/teleport
…component/teleport
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Trigger (slot)
participant TrBasePopper
participant TrTeleport
participant PopperContent (slot)
User->>Trigger (slot): Clicks trigger element
Trigger (slot)->>TrBasePopper: Receives trigger event
TrBasePopper->>TrBasePopper: Computes position & visibility
TrBasePopper->>TrTeleport: Teleports popper content to target
TrTeleport->>PopperContent (slot): Renders dropdown menu
User->>Trigger (slot): Clicks outside
TrBasePopper->>TrBasePopper: Hides popper
Poem
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- Added a new `BasePopper` component to manage popper positioning and visibility. - Refactored `DropdownMenu` to utilize `BasePopper` for improved structure and event handling. - Removed unused properties and simplified the dropdown menu's implementation. - Enhanced styling and slot support for better customization.
gene9831
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/components/src/base-popper/index.vue (3)
1-1: Consider removing the ESLint disable comment.The
@typescript-eslint/no-explicit-anydisable comment covers the entire file. Consider using more specific type annotations instead ofanyto improve type safety.
116-151: Sophisticated positioning logic with viewport constraint handling.The positioning implementation is well thought out:
- Uses CSS
clamp()for elegant viewport boundary constraints- Handles all placement combinations correctly
- Properly accounts for both main and cross axis offsets
One minor suggestion for line 138 - the bottom calculation could be simplified for better readability.
Consider extracting the complex calculation on line 138 into a computed property or helper function for better readability:
if (placement.includes('bottom')) { - set('bottom', `calc(100% - ${toCssUnit(bottom.value + popperHeight.value + resolvedOffset.value.mainAxis)})`) + const bottomPosition = bottom.value + popperHeight.value + resolvedOffset.value.mainAxis + set('bottom', `calc(100% - ${toCssUnit(bottomPosition)})`) }
172-186: Consider adding ARIA attributes for accessibility.While the component handles positioning well, it lacks accessibility attributes that would help screen readers understand the relationship between triggers and popper content.
Consider adding ARIA attributes to establish the relationship between triggers and content:
<component v-for="(vnode, index) in renderedTriggerVNodes" :key="(vnode as VNode).key ?? index" :is="vnode" :ref="(el: unknown) => setRefs(el, index)" v-bind="indexedEventHandlers[index]" + :aria-expanded="show" + :aria-controls="show ? `popper-${$.uid}` : undefined" /> <TrTeleport :anchor="unrefElement(triggerRef)"> <Transition v-bind="transitionProps"> - <div v-if="show" class="tr-base-popper" ref="popperRef" :style="popperStyles" v-bind="$attrs"> + <div v-if="show" :id="`popper-${$.uid}`" class="tr-base-popper" ref="popperRef" :style="popperStyles" v-bind="$attrs" role="tooltip"> <slot name="content" /> </div> </Transition> </TrTeleport>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/demos/dropdown-menu/basic.vue(2 hunks)docs/demos/suggestion/pills-popper.vue(1 hunks)packages/components/src/base-popper/index.ts(1 hunks)packages/components/src/base-popper/index.vue(1 hunks)packages/components/src/dropdown-menu/index.type.ts(2 hunks)packages/components/src/dropdown-menu/index.vue(5 hunks)packages/components/src/suggestion-pills/components/PillButtonWrapper.vue(1 hunks)packages/components/src/suggestion-pills/index.vue(1 hunks)packages/components/src/teleport/index.ts(1 hunks)packages/components/src/teleport/index.vue(1 hunks)packages/components/tsconfig.json(0 hunks)packages/components/vite.config.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/components/tsconfig.json
- packages/components/vite.config.ts
🔇 Additional comments (11)
packages/components/src/teleport/index.ts (1)
1-1: Verify the referenced Vue component exists.The export pattern follows standard conventions for Vue component libraries. However, ensure that
./index.vueexists and properly implements the TrTeleport component.#!/bin/bash # Description: Verify that the TrTeleport Vue component exists and is properly implemented # Expected: Find the index.vue file and verify it's a valid Vue component # Check if the Vue component file exists fd -t f "index.vue" packages/components/src/teleport/ # If found, examine the component structure if [ -f "packages/components/src/teleport/index.vue" ]; then echo "=== TrTeleport Component Content ===" cat packages/components/src/teleport/index.vue else echo "ERROR: index.vue not found in teleport directory" fipackages/components/src/base-popper/index.ts (1)
1-5: Component export pattern looks good.The implementation follows standard Vue component library patterns:
- Proper import/export structure
- Consistent naming convention with 'Tr' prefix
- Explicit component name assignment for debugging
#!/bin/bash # Description: Verify that the BasePopper Vue component exists and follows expected patterns # Expected: Find the index.vue file and verify it's a valid Vue component with proper structure # Check if the Vue component file exists if [ -f "packages/components/src/base-popper/index.vue" ]; then echo "=== BasePopper Component Found ===" # Check for basic Vue component structure head -20 packages/components/src/base-popper/index.vue echo "..." echo "=== Component structure analysis ===" rg -A 5 -B 5 "<template|<script|export default" packages/components/src/base-popper/index.vue else echo "ERROR: index.vue not found in base-popper directory" fidocs/demos/dropdown-menu/basic.vue (1)
3-5: Consistent trigger slot implementation across different trigger modes.The updates correctly implement the new trigger slot pattern for both automatic and manual trigger modes. The slot usage is consistent and maintains proper event handling for the manual trigger case.
Also applies to: 15-17
packages/components/src/suggestion-pills/components/PillButtonWrapper.vue (1)
37-39: Precise and targeted API migration.The change correctly updates only the dropdown menu case while preserving:
- Conditional rendering logic for different action types
- Event handling (
@pointerupemission)- Component props and styling
This demonstrates careful attention to the component's existing behavior patterns.
packages/components/src/suggestion-pills/index.vue (1)
117-120: Good conditional check for click-outside event emission.The addition of the
showAll.valuecheck ensures that the click-outside event is only emitted when the component is in the expanded state. This prevents unnecessary event propagation when the component is collapsed.packages/components/src/dropdown-menu/index.type.ts (1)
1-2: Type definitions correctly updated for the new slot-based trigger pattern.The changes properly reflect the dropdown menu's new architecture:
- Import of
VNodetype for proper slot typing- Slot renamed from
defaulttotriggerwith appropriate VNode return typeThese align well with the component's refactor to use
TrBasePopper.Also applies to: 23-24
packages/components/src/teleport/index.vue (1)
1-26: Well-implemented teleport wrapper with Shadow DOM support.This component elegantly handles Shadow DOM compatibility by:
- Detecting when the anchor element is within a Shadow DOM tree
- Automatically teleporting to the ShadowRoot when appropriate
- Falling back to document.body for regular DOM contexts
The implementation is clean and focused on its single responsibility.
packages/components/src/dropdown-menu/index.vue (2)
1-84: Excellent refactor to use TrBasePopper for positioning logic.The refactor successfully:
- Delegates complex positioning calculations to a dedicated component
- Simplifies the dropdown menu to focus on its core functionality
- Maintains all existing behavior while improving maintainability
- Properly handles manual vs automatic show state control
The use of computed refs to access basePopper's internal elements is a clean approach.
86-136: Styles properly organized for the refactored component.Good separation of:
- Non-scoped styles for the popper container (transition animations)
- Scoped styles for the dropdown menu list items
The transition timing and visual styling are preserved from the original implementation.
packages/components/src/base-popper/index.vue (2)
44-84: Robust trigger reference handling with good edge case coverage.The implementation correctly:
- Handles Fragment nodes from v-for loops
- Supports both Vue component instances and native DOM elements
- Provides helpful console warnings for invalid trigger types
- Manages dynamic trigger counts with proper cleanup
85-99: Clean implementation of indexed event handlers.The event handler wrapping correctly:
- Preserves the original event handler pattern (onXxx)
- Passes the trigger index as the last argument
- Only wraps actual functions
This enables proper multi-trigger support.
- Replaced the custom `TrTeleport` component with the new `Teleport` functionality in `BasePopper` for improved performance and simplicity. - Introduced a new `useTeleportTarget` composable to manage teleport target selection based on the reference element. - Refactored `DropdownMenu` styles to reset padding and margin for better layout control. - Removed deprecated `teleport` component files to streamline the codebase.
- Removed unused ComputedRef type from the `useTeleportTarget` function signature for cleaner code. - Streamlined the function definition to enhance readability and maintainability.
2 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores