feat(FilePreview): add component #1880
Conversation
KirillDyachkovskiy
requested review from
amje,
ValeraS and
korvin89
as code owners
|
Preview is ready. |
|
Visual Tests Report is ready. |
| height: 64px; | ||
| width: 96px; | ||
|
|
||
| &-img { |
There was a problem hiding this comment.
It looks like separated block &__image-img (with confusing naming). Could you move it out from the &__image block?
src/components/FilePreview/types.ts
Outdated
There was a problem hiding this comment.
You can describe FileType and FILE_TYPES in a more flexible way:
export const FILE_TYPES = [
'default',
'image',
'video',
'code',
'archive',
'music',
'text',
'pdf',
'table',
] as const;
export type FileType = (typeof FILE_TYPES)[number];|
|
||
| const handleClick: React.MouseEventHandler<HTMLDivElement> = (e) => { | ||
| if (mobile && isPreviewString && !onClick) { | ||
| setShowPreviewSheet(true); |
There was a problem hiding this comment.
Why onClick does not triggered here? It looks inconsistent
|
|
||
| export interface FilePreviewActionProps { | ||
| id?: string; | ||
| icon: IconData; |
There was a problem hiding this comment.
Why it cannot be just a react node?
| }: FilePreviewActionProps) { | ||
| return ( | ||
| <ActionTooltip id={id} title={title} {...tooltipExtraProps}> | ||
| <Button |
There was a problem hiding this comment.
Lets add aria-describedby to the Button and id to the tooltip
src/components/FilePreview/README.md
Outdated
|
|
||
| ### Properties | ||
|
|
||
| | Property | Type | Required | Default | Description | |
There was a problem hiding this comment.
We use this order of columns for consistency purposes
src/components/FilePreview/index.ts
Outdated
| @@ -0,0 +1,4 @@ | |||
| export {FilePreview} from './FilePreview'; | |||
| export type {FilePreviewProps} from './FilePreview'; | |||
| export {getFileType} from './utils'; | |||
There was a problem hiding this comment.
Why we need it as a public interface?
KirillDyachkovskiy
force-pushed
the
uikit-file-preview
branch
from
9c4a2ac to
6168e5e
Compare
|
@korvin89 please come back to this PR, Kirill added 4 commits after the most recent comments, i think he implemented your suggestions. |
Component moved from @gravity-ui/components