Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Picker): support 'root' attribute #1442

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/react-components/src/components/Picker/Picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const Picker: React.FC<IPickerProps> = ({
onSelect,
placement,
floatingStrategy,
root,
useDismissHookProps,
useClickHookProps,
virtuosoProps,
Expand Down Expand Up @@ -149,7 +150,7 @@ export const Picker: React.FC<IPickerProps> = ({
</PickerTrigger>
<FloatingNode id={nodeId}>
{isOpen && (
<FloatingPortal>
<FloatingPortal root={root}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Add story for custom portal root

The root prop is properly typed and implemented, but there's no story demonstrating its usage. Current stories don't cover portal behavior customization.

  • Add a new story in Picker.stories.tsx showcasing the root prop with a custom container element
  • Include examples of portal mounting to different DOM elements
🔗 Analysis chain

Verify portal behavior with custom root

The implementation looks correct. Consider adding a Storybook story that demonstrates the root prop functionality with different root elements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there's a corresponding story for root prop
rg -l "root.*?FloatingPortal" "packages/react-components/src/components/Picker/**/*.stories.tsx"

Length of output: 203


Script:

#!/bin/bash
# Let's first check if stories exist in the correct location
fd "stories.tsx" packages/react-components/src/components/Picker/

# Check for any stories related to Picker component
fd "stories.tsx" . | grep -i "picker"

# Let's also verify the actual implementation of FloatingPortal usage
rg "FloatingPortal" packages/react-components/src/components/Picker/

Length of output: 951


Script:

#!/bin/bash
# Let's check the content of the Picker stories to see if root prop is covered
cat packages/react-components/src/components/Picker/Picker.stories.tsx

# Also check the types file to understand root prop definition
cat packages/react-components/src/components/Picker/types.ts

Length of output: 13484

<PickerList
pickerType={type}
options={items}
Expand Down
5 changes: 5 additions & 0 deletions packages/react-components/src/components/Picker/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { InputHTMLAttributes, ReactElement } from 'react';

import {
FloatingPortalProps,
Placement,
Strategy,
UseClickProps,
Expand Down Expand Up @@ -132,6 +133,10 @@ export interface IPickerProps extends ComponentCoreProps {
* Floating strategy for the picker component from @floating-ui/react
*/
floatingStrategy?: Strategy;
/**
* Root node the portal container from @floating-ui/react will be appended to
*/
root?: FloatingPortalProps['root'];
/**
* Set the `floating-ui` useDismiss hook params if you need more control
* https://floating-ui.com/docs/usedismiss
Expand Down
Loading