-
Notifications
You must be signed in to change notification settings - Fork 126
feat(combobox): add ghost variant and clickable chevron #2880
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
Conversation
Add transition-transform class and conditional rotate-180 transform to the chevron icon in menu button for improved visual feedback when expanding/collapsing menus.
- Add ghost variant with transparent borders and hover effects - Add showChevron prop with animated rotation on open/close - Chevron is clickable to toggle combobox state - Include stories for both new features - Maintain backward compatibility
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…vron - Replace chevron-up/chevron-down icon switching with rotation animation - Use chevron-down as base icon and rotate 180 degrees when open - Consistent with combobox implementation and smoother animation
…t variant - Add configurable paddingX and paddingXMd props to top-nav with defaults - Update combobox ghost variant to not change color on focus - Use lighter hover color (surface-interactive-secondary) for better dark mode contrast
Reorganize Tailwind classes in top-nav component with logical grouping: - Surface styling (surface-primary) - Positioning (sticky top-0 z-40) - Display (hidden md:flex) - Layout (w-full, flex-col md:flex-row) - Alignment (items-center justify-end) - Borders (border-b border-subtle) - Padding (p-1 px-4 md:px-8) - Custom class override (className) Also added missing twMerge import for proper class merging.
| <button | ||
| type="button" | ||
| class="hover:bg-gray-100 flex h-full items-center rounded pr-2 focus:outline-none" | ||
| on:click|stopPropagation={() => ($open ? closeList() : openList())} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we stop propagation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ternaries give me the ick I think I'd prefer an if statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gonna have to get ourselves in a cage and duke this out. Much prefer this over an if statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like little ternaries. the big ones or nested ones hurt my feelings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about a toggleList() function, no ternary needed 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we stop propagation here?
Do we want the event bubbling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about a
toggleList()function, no ternary needed 🤔
hmm. I could add a toggle but its going to be doing essentially the same thing since the open and close callbacks do more than simply set state.
| @apply opacity-50; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolute cinema
- Add ClassNameValue type to top-nav for better type safety - Refactor combobox chevron rotation to use twMerge for cleaner conditional classes - Replace template literal class concatenation with explicit boolean conditions
Summary
Enhanced UI components with new styling variants, configuration options, and improved maintainability.
Changes
Combobox Enhancements
variant="ghost"prop for borderless appearance with transparent backgroundsurface-interactive-secondaryfor improved contrastshowChevron={true}prop that displays an animated chevron iconTop Navigation Improvements
paddingXandpaddingXMdprops with backward-compatible defaultsMenu Button Enhancement
Features
Ghost Variant
Animated Chevron
Configurable Navigation
Backward Compatibility
All changes are additive and maintain full backward compatibility. Existing component usage remains unchanged with sensible defaults.
Test Plan