COM-357: Add support for additional content in DAM grid rows #1697
Conversation
We need this for the "External DAM systems" feature to show the import source in the grid.
| additionalToolbarItems?: React.ReactNode; | ||
| renderAdditionalRowContent?: (row: DamRow) => React.ReactNode; |
There was a problem hiding this comment.
I decided to add the render prefix because this is a render prop. However, it's kinda odd that additionalToolbarItems isn't a render prop. What do you think?
There was a problem hiding this comment.
yes, they could both use the same technique
There was a problem hiding this comment.
What do we prefer? We currently have three different techniques in our code base:
- Component Type (allows direct use of hooks): https://github.com/vivid-planet/comet/blob/main/demo/admin/src/pages/PageContentBlock.tsx#L41
- render-Prop: https://github.com/vivid-planet/comet/blob/main/demo/admin/src/common/MasterMenu.tsx#L102
- ReactNode:
additionalToolbarItems
yes, they could both use the same technique
I can't really use ReactNode here for both cases because I need to pass row
There was a problem hiding this comment.
what is the difference between the first two?
| @@ -51,6 +51,7 @@ export { useDamConfig } from "./dam/config/useDamConfig"; | |||
| export { useCurrentDamFolder } from "./dam/CurrentDamFolderProvider"; | |||
| export { DamPage } from "./dam/DamPage"; | |||
| export { useDamFileUpload } from "./dam/DataGrid/fileUpload/useDamFileUpload"; | |||
| export type { DamRow } from "./dam/DataGrid/FolderDataGrid"; | |||
There was a problem hiding this comment.
imho that name is too generic (for a public export)
DamRowFragment? DamRowData?
There was a problem hiding this comment.
Maybe DamGridItem or DamGridRow?
There was a problem hiding this comment.
it's data structure (that contains just the data) vs. component (that renders it)
usually our exports are components, so I could think DamGridRow is also a component
There was a problem hiding this comment.
I could simply re-export the generated GraphQL type without a renaming, what do you think?
There was a problem hiding this comment.
The reason why you add it to the public api is, because it gets passed in renderAdditionalRowContent render prop.
final-form names the arguments passed to a render function eg FieldRenderProps, maybe you could use AdditionalRowContentRenderProps accordingly?
|
Did you also consider additional columns? |
No I haven't. I believe it will increase complexity even more than the additional content 🤔 |
well, the resulting UI should usually not depend on how complex it was to implement :) |
You're right, I've asked UX about it and will update the PR depending on the outcome. |
|
Superseded by #1756. |
The import source column is only shown if `importSources` is provided via `DamConfigProvider`. This is done to not show the column if no external DAMs are supported. --- <img width="1304" alt="image" src="https://github.com/vivid-planet/comet/assets/48853629/33e9fb39-49a3-429b-bea5-082c7f9d8bdd"> - [x] Add changeset (if necessary) Alternative to #1697
We need this for the "External DAM systems" feature to show the import source in the grid.