feat(FormBlock): add content labels, stub, update background #1314
Conversation
|
Preview is ready. |
|
Playwright Test Component is ready. |
PahaN47
changed the title
| Device.Desktop in value && | ||
| Device.Mobile in value |
There was a problem hiding this comment.
Maybe need "or" between this conditions? Does it always have to be desktop and mobile at one time?
src/blocks/Form/__stories__/Form.mdx
Outdated
| } | ||
| ``` | ||
|
|
||
| `stub?: React.ReactNode` - A stub that will be rendered instead of the form. |
There was a problem hiding this comment.
Maybe name it "customFormComponent"?
|
|
||
| const b = block('content-labels'); | ||
|
|
||
| const ContentLabels = ({ |
There was a problem hiding this comment.
Why don't we use a component from uikit?
https://preview.gravity-ui.com/uikit/?path=/story/components-data-display-label--icon
There was a problem hiding this comment.
It does not seem to fit all that well: here the labels need to be larger and the colors would have to be overridden manually anyway. All in all, using the uikit component wouldn't change the amount of required custom styles much and, I figured, could introduce some bugs with theming and such
|
|
||
| export enum Device { | ||
| Desktop = 'desktop', | ||
| DesktopSmall = 'desktopSm', |
There was a problem hiding this comment.
Is this agreed with the designer and owners?
@NikitaCG
| { | ||
| type: 'object', | ||
| additionalProperties: false, | ||
| required: [Device.Desktop, Device.Mobile], |
There was a problem hiding this comment.
Why is it required? We can setup image only one device
There was a problem hiding this comment.
The thought process was: if you want the same background for everything, you do not use device-specific values, if you want to have a background for a single breakpoint range, then you could specify something like background: none for the rest
No description provided.