feat(Overlay): implement component #1474
Conversation
|
Preview is ready. |
|
Playwright Test Component is ready. |
| </div> | ||
| ); | ||
| }; | ||
| export const Default = DefaultTemplate.bind({}); |
There was a problem hiding this comment.
pls add more examples in storybook.
I think overlay in dialog content and table overlay would be enough.
It is better to do an example with a table when there is no data at first and they are loaded worse to show how overlay min height work
There was a problem hiding this comment.
👌 Added some additional examples
| } | ||
|
|
||
| export function LoadingOverlay({ | ||
| loaderProps, |
There was a problem hiding this comment.
do we realy need override className in Loader component?
I think it's not worth giving the user this opportunity to mix class name.
instead of loaderProps let pass size prop only?
There was a problem hiding this comment.
Removed the Loader from here after the discussion, so looks like it's no longer relevant :)
| <Box className={b(null, className)} {...boxProps}> | ||
| <div className={b('overlay', {visible: loading})}> | ||
| <div className={b('background', {type: background})} /> | ||
| <Loader {...loaderProps} /> |
There was a problem hiding this comment.
how do you think, may be add ability to pass a component insted of hardcode Loader component?
const LoadingOverlay = ({
loaderSlot,
size = 'm',
}: {loader: React.ReactNode}) => {
return (
...
{typeof loaderSlot === 'undefined' ? <Loader size={size} /> : loaderSlot}
...
)
}it is important to make it possible to pass null so that the display can be completely redefined.
| <div>I am an example</div> | ||
| <div>content</div> | ||
| </LoadingOverlay> | ||
| ``` |
There was a problem hiding this comment.
need to add some info about override background by --g-color-base-background css custom property
and examples with loader overrides
There was a problem hiding this comment.
Added examples for different loaders, as for overriding --g-color-base-background, as I remember, we've decided that it's not really appreciated by designers?
Vladeeg
force-pushed
the
feature/implement-loading-overlay-component
branch
from
0368a2c to
73868ac
Compare
Vladeeg
changed the title
Vladeeg
force-pushed
the
feature/implement-loading-overlay-component
branch
2 times, most recently
from
9de529e to
792bbf6
Compare
src/components/Overlay/Overlay.tsx
Outdated
| children, | ||
| }: OverlayProps) { | ||
| return ( | ||
| <div className={b({visible: loading}, className)}> |
There was a problem hiding this comment.
You can use Flex component here and remove styles with centring from .scss file
src/components/Overlay/README.md
Outdated
| ### Basic usage | ||
|
|
||
| ```jsx | ||
| <div style={{position: 'relative'}}> |
There was a problem hiding this comment.
When we discussed this component last time, we agreed to add the position property to the Box component and make an example with this application
src/components/Overlay/README.md
Outdated
| | view | `"base"` `"float"` | | `base` | Overlay background style | | ||
| | children | `React.ReactNode` | | | Content, usually a Loader component | | ||
|
|
||
| ### Basic usage |
There was a problem hiding this comment.
Can use also add .msx docs for storybook?
src/components/Overlay/README.md
Outdated
| import {Overlay} from '@gravity-ui/uikit'; | ||
| ``` | ||
|
|
||
| The `Overlay` component renders an overlay over the parent element with relative position. |
There was a problem hiding this comment.
it should be highlighted here that the component should be used with a parent with position: relative
src/components/Overlay/Overlay.tsx
Outdated
| children?: React.ReactNode; | ||
| } | ||
|
|
||
| export function Overlay({ |
There was a problem hiding this comment.
Backdrop is a special case of Overlay use. So I think Overlay name is ok
| <div style={{position: 'relative'}}> | ||
| <div>Some content to hide under overlay</div> | ||
| <Overlay visible={loading}> | ||
| <Loader /> |
There was a problem hiding this comment.
Yes, I believe it's better to be that way, Overlay is a utility component, it shouldn't have "default" look. The look will depend on the usage context. Also, turning off the default behaviour would look a bit wierd:
<Overlay visible children={null} />
Vladeeg
force-pushed
the
feature/implement-loading-overlay-component
branch
from
792bbf6 to
9fe109d
Compare
Vladeeg
force-pushed
the
feature/implement-loading-overlay-component
branch
from
9fe109d to
207be73
Compare
|
Visual Tests Report is ready. |
Vladeeg
force-pushed
the
feature/implement-loading-overlay-component
branch
from
97fcca7 to
336ec67
Compare
No description provided.