-
Notifications
You must be signed in to change notification settings - Fork 5
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
Palette-based color picker #6028
Conversation
const isTouch = (e: InteractionEvent): e is TouchEvent => 'touches' in e; | ||
const clamp = (x: number, min = 0, max = 1) => | ||
x > max ? max : x < min ? min : x; | ||
const useIsomorphicLayoutEvent = |
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 can't see where this is going to be useful? useLayoutEffect
is only ever run on the client?
Unless the check is doing something that isn't that?
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.
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.
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.
Interesting, I wonder if we could just use useLayoutEffect
though. Happy for either.
common/views/components/PaletteColorPicker/PaletteColorPicker.tsx
Outdated
Show resolved
Hide resolved
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.
Loving this - I also like that this could easily be implemented sans-JS.
This is an (arguably) improved UI for colour filtering based on the design in Zeplin.
The library we're using doesn't export the plumbing that we want so I replicated it (ish) which makes the UI much better suited to our needs (and doesn't cause CSS import problems).
It looks like this: