feat(ColorsGrid): add ColorsGrid component #1178
Conversation
|
Playwright Test Component is ready. |
|
Preview is ready. |
| @@ -0,0 +1,85 @@ | |||
| @use '../variables'; | |||
|
|
|||
| $block: '.#{variables.$ns}colors-grid'; | |||
There was a problem hiding this comment.
Let's use $ns-new for newer components
| --yc-color-size: 28px; | ||
| --yc-color-icon-size: 12px; | ||
| --yc-color-gap: 1px; |
There was a problem hiding this comment.
If these are considered public, then please follow this pattern for variables
|
|
||
| width: min-content; | ||
| display: grid; | ||
| grid-template-columns: repeat(var(--cols-num), 1fr); |
There was a problem hiding this comment.
"private" vars should starts with --_--
| &__check { | ||
| width: var(--yc-color-icon-size); | ||
| height: var(--yc-color-icon-size); | ||
| z-index: 1; |
There was a problem hiding this comment.
Why we need these z-index: 1 through the file?
There was a problem hiding this comment.
So that the hover styles of the next element do not overlap with this one
| @@ -0,0 +1,82 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
What about changing name to the ColorPalette?
| export interface ColorsGridProps extends DOMProps, QAProps { | ||
| colors: Color[]; | ||
| value?: string; | ||
| rowSize?: number; |
There was a problem hiding this comment.
Can we add rows, columns props to separately configure grid?
| return ( | ||
| <div | ||
| key={index} | ||
| data-value={item.value} |
There was a problem hiding this comment.
We don't need data-attributes. Instead we can pass the value directly to the onClick handler
| key={index} | ||
| data-value={item.value} | ||
| role={isClickable ? 'button' : undefined} | ||
| tabIndex={isClickable ? 0 : undefined} |
There was a problem hiding this comment.
I think the component's keyborad interaction should be the same as in radiogroups. It can also use radio inputs for "native" interactions
There was a problem hiding this comment.
In my case, a request is sent when choosing a color. Native navigation will cause a lot of queries
| {value: 'color_12', color: '#52A6C5'}, | ||
| ]; | ||
|
|
||
| const DefaultTemplate: StoryFn<ColorsGridProps> = (args) => { |
There was a problem hiding this comment.
Let's use newer story syntax. See example in Button
| element.style.color = color; | ||
| document.body.appendChild(element); | ||
|
|
||
| const computedColor = window.getComputedStyle(element).color; |
There was a problem hiding this comment.
Why we can't use elements from the component itself?
There was a problem hiding this comment.
It is necessary to calculate the color before the first display of the elements. Doing this in useEffect after the first render doesn't seem like a good idea
aulian0v
force-pushed
the
feat/colors-grid
branch
from
7473d2e to
db26335
Compare
|
You can now use the |
No description provided.