-
Notifications
You must be signed in to change notification settings - Fork 25
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: pretty filter labels #2911
Conversation
… components, initialize yarn workspace for Filters, initialize FiltersList and FilterMenu components, add licenses, stub out files needed for generating READMEs for FilterMenu and FiltersList
* feat(local only storybook): add ability to only display a story in the sidebar when running storybook locally by adding a 'local-dev' tag to the story meta * feat(local only storybook): use tag in mdx files as well * feat(local only storybook): add local story stubs for FiltersList, add documentation for local-dev tag in storybook/README.md * fix(tag): remove local-dev tag from dropdownmenu readme
…in storybook/package.json in order to merge local dev tagging to filters files branch
…or effective component API
…ader, update conditional onclicks to give an alert for POC purposes
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
2274a50
to
e36c19f
Compare
import { Footer } from './footer'; | ||
import { Header } from './header'; | ||
import { TriggerButton } from './trigger-button'; | ||
import * as Popover from '@radix-ui/react-popover'; |
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'm really happy with how radix handles the popover
compared to the DropdownMenu
. I think we should use it.
I also think we should consider replacing the DropdownMenu
internals with the radix popover
instead of trying to implement complicated bugfixes and a11y updates to it ourselves.
const { colorNeutral85, colorNeutral40, colorPrimary, fontSize20 } = | ||
designTokens; | ||
|
||
const useScrollObserver = (ref: RefObject<HTMLElement>, totalCount: number) => { |
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.
This is a really elegant solution to hiding Chip
s and displaying a count of how many Chip
s are hidden in the Badge
.
height: '2rem', | ||
display: 'flex', | ||
alignItems: 'center', | ||
background: 'linear-gradient(to right, transparent, white 25%)', |
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.
the gradient is a nice touch
</ClearButtonContainer> | ||
|
||
<CaretDownIcon size="small" color="neutral60" /> | ||
<OverlayButton ref={ref} aria-label={label} {...rest} /> |
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.
This is a good solution to not requiring nested buttons
<div | ||
style={{ | ||
display: 'flex', | ||
flexDirection: 'row-reverse', |
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.
Reversing the chips so that the most recently selected value is displayed first leads to some unexpected behavior in which the count becomes unreliable, as shown in the screen grab below:
Screen.Recording.2024-09-24.at.2.32.39.PM.mov
This is the one update in this POC that I don't think we should use in the FilterMenu
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 reverted the reversing
This reverts commit bb8a5fa.
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 really love how clean the design looks with this approach, the fade in, gradient, it all works well together. I looked over the Decisions Made section of the ticket and agree with it all.
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.
Love the slick UI of this.
closed because this is a PoC, will use for reference |
creating a draft to have a storybook which displays the current filter status