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

Fix long list output in Dropdown #535

Merged
merged 3 commits into from
Feb 4, 2025
Merged
Changes from 2 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
4 changes: 4 additions & 0 deletions src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const Dropdown = (props: DropdownMenu.DropdownMenuProps) => (
const DropdownMenuItem = styled(GenericMenuItem)`
position: relative;
display: flex;
min-height: 32px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we adding min-height here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vineethasok I found that just adding overflow-y: auto on the parent container was not enough to maintain the expected height of the Dropdown.Item elements.

I think it might be because there is a max expected height of the parent container and the height of this child component is calculated based on that; the more children, the less their height to fit inside the parent

&[data-state="open"] {
${({ theme }) => `
font: ${theme.click.genericMenu.item.typography.label.hover};
Expand Down Expand Up @@ -89,10 +90,13 @@ type DropdownSubContentProps = DropdownMenu.MenuSubContentProps &
MainDropdownProps &
ArrowProps;

console.log("Max height: ", )
const DropdownMenuContent = styled(GenericMenuPanel)`
min-width: ${({ theme }) => theme.click.genericMenu.item.size.minWidth};
Copy link
Collaborator

Choose a reason for hiding this comment

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

@natalyjazzviolin Maybe we can also add maxHeight as a prop which can be provided by the user
@gjones Would love your thoughts

flex-direction: column;
z-index: 1;
overflow-y: scroll;
max-height: calc((var(--radix-${({ $type }) => $type}-content-available-height) - 100px))
`;

const DropdownContent = ({
Expand Down