#433: Add EbayFilePreviewCard #475
Conversation
🦋 Changeset detectedLatest commit: 41ea3e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bruno2kd
changed the title
src/ebay-file-preview-card/README.md
Outdated
| | ---------------------- | ------------------------------------------------------------ | -------- | --------------------------------------------------------------- | ---- | | ||
| | `a11yCancelUploadText` | `String` | No | a11y text for cancel upload button. | | | ||
| | `deleteText` | `String` | No | Text for delete button. | | | ||
| | `status` | `'uploading'` | No | Status of the file, can be `"uploading"` or `undefined`. | | |
There was a problem hiding this comment.
We usually use type String even it is an option. We are planning to update the docs and have a Options column, but that is being part of a different PR, also no need to pass undefined in the description
src/ebay-file-preview-card/README.md
Outdated
| | `labelText` | `String` | No | Text to display in the label. | | | ||
| | `footerTitle` | `String` | No | Title to display beneath the file, usually the filename. | | | ||
| | `footerSubtitle` | `String` | No | Subtitle to display beneath the file title. | | | ||
| | `file` | `File` or `{name: string, type?: File[type], src?: string }` | No | If true, combobox will span the entire width of it's container. | | |
There was a problem hiding this comment.
The description of file looks incorrect, you can check the Marko description and use the same
src/ebay-file-preview-card/README.md
Outdated
|
|
||
| | Name | Type | Required | Description | Data | | ||
| | -------------- | ---------------------------- | -------- | --------------------------------------------------- | ---- | | ||
| | `onMenuAction` | `EbayMenuSelectEventHandler` | No | Triggered when an action is selected from the menu. | | |
There was a problem hiding this comment.
We need to pass the object that we are passing in the Data column. And it should be part of the Attributes. Follow the combobox documentation as an example
src/ebay-file-preview-card/types.ts
Outdated
| onCancel?: EbayEventHandler | ||
| } | ||
|
|
||
| type SelectProps = { |
There was a problem hiding this comment.
This should be called MenuActionEventData
| } | ||
|
|
||
| let fileLabel | ||
| if (previewFile && previewFile?.type !== 'image') { |
There was a problem hiding this comment.
Since optional chaining is being used there is no need for previewFile &&
There was a problem hiding this comment.
I'll do previewFile && previewFile.type, otherwise ts complains
| }) => { | ||
| const previewFile = useMemo(() => { | ||
| if (!rawFile) return undefined | ||
| let file = (rawFile || {}) as Exclude<typeof rawFile, File | undefined> |
There was a problem hiding this comment.
there is no need for || {} since you are filtering it already on line 27. If we want to have the same logic as marko, the condition on line 27 needs to be removed
| </EbayMenuButtonItem> | ||
| ))} | ||
|
|
||
| <EbayMenuButtonItem value="delete"> |
There was a problem hiding this comment.
We might need to add a key="delete" here
| } | ||
| } | ||
|
|
||
| export const Image = { |
There was a problem hiding this comment.
We should follow same structure as other components for story definition (check combobox)
// Not the usage of a function instead of { render }
export const Default: StoryObj<typeof EbayFilePreviewCard> = (args) => (
)And there is no need for fragments in case one single element is rendered. make sure to add types for all the stories
| type: 'image', | ||
| src: 'https://ir.ebaystatic.com/cr/v/c01/skin/docs/tb-real-square-pic.jpg' | ||
| }} | ||
| onCancel={(e) => action('onCancel')(e)} |
There was a problem hiding this comment.
These are already handle by ...args no need to add them
| } | ||
| return ( | ||
| <CardEl className="file-preview-card" {...rest}> | ||
| <div className="file-preview-card__body">{previewCardContent}</div> |
There was a problem hiding this comment.
These logic is quite tricky to read. I think we can separate into two components instead. At the end we will use something like these bellow (just to show the idea)
return (
...
<div class="file-preview-card__body">
<FileContent file={file} status={status} isFade={seeMore !== undefined} />
{seeMore ? (
<button></button>
) : <FileAction file={file} infoText={infoText} status={status} onCancel menuActions onMenuSelect onDelete />
)Then each component can have the logic inside it
export const FileContent = ({}) => {
if (status === uploading) {
return ()
}
switch(file.type) {
case 'video':
return <video />
case 'image':
return <img />
default:
return <EbayIcon />
}
}Same for the action. I think this is a more "react" way of solving the problem
* add ebay preview card component * add changeset * add component to library readme * update tests * fix types issues * improve documentation * fix components rendering logic * update storybook * export default meta storybook * refactor component to have different componens * handle menu select on preview action component --------- Co-authored-by: Bruno Kern <bruduarte@ebay.com>
* add ebay preview card component * add changeset * add component to library readme * update tests * fix types issues * improve documentation * fix components rendering logic * update storybook * export default meta storybook * refactor component to have different componens * handle menu select on preview action component --------- Co-authored-by: Bruno Kern <bruduarte@ebay.com>
Issue: #433
Demo:
Screen.Recording.2025-04-06.at.21.27.52.mov
Coverage: