feat(dataspreadsheet): optional custom component to row header #5500
Conversation
AustinGitHub
requested review from
davidmenendez and
annawen1
and removed request for
a team
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Please let me know if there is better way to name the variables and descriptions. I was unsure on those, thanks! Confirmed with my teams UX that this is good for us. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@AustinGitHub thanks for the submission. i'll need to confirm with design that this enhancement is acceptable. |
|
oh ok i see it under the usage guidelines. i'll go through the code now 👍 |
| data={data} | ||
| onDataUpdate={setData} | ||
| rowNumberingComponentPosition="Left" | ||
| rowNumberingComponent={buildComponent} |
There was a problem hiding this comment.
i think rowNumberingComponent is a tad bit confusing. i think that because the documentation specifies usage of overflow menu that maybe we do an implementation where the API is called rowActions which accepts an array of actions that can be mapped over internally.
There was a problem hiding this comment.
So does the documentation specify it has to be overflow menu? I have it set so user provides custom component so it could be icon, button, or overflow menu.
There was a problem hiding this comment.
it's a bit hard to find because it's nested under an accordion, but it seems like it's mostly targeted towards overflow usage.
actually looking at this more i think maybe this should be set in the columns API. something like
const columnData = [
{
Header: 'Row Index',
headerActions: [{ label: 'delete', onClick: () => {} }, ...],
accessor: (row, index) => index,
...
let me pull in @matthewgallo on this since he's much more familiar with this component and i'd like to get his take on this.
There was a problem hiding this comment.
after talking it over with @matthewgallo we think that a new prop called renderHeaderOverflow would work for this. the only main change we'd like to see is that this overflow is also accessible in the column header as per the usage guidelines. the prop could also provide a callback that distinguishes between a row and column header in case a separate set of actions are required for rows and columns. does that make sense?
There was a problem hiding this comment.
Result
Edit: I will discuss with Matt to see what should be done. Thanks!
There was a problem hiding this comment.
Hi @davidmenendez I discussed with Matt on this with some back and forth to get understanding.
Currently, with column headers user can already provide custom component to it as with the example I provided above with how we have this in our code right now.
For row headers on the row indexing numbering column there is no user control on this as it is hardcoded in. I explained to Matt how it is easier for Developer to provide their own custom component such as the overflow menu and if that menu has action events, it is easier for us to handle these action events rather than having to try and get them sorted through carbon products.
The changes I will be making is renaming the components to something more understandable like renderRowHeader and renderRowHeaderDirection. I'll also double check with the carbon guidelines, but storybook does have it's own section showing user how to utilize this feature and example is going to be using the overflowmenu.
In future, if this is wanting to be changed around then that is fine but for now since column headers is already customizable, rather than making this not customizable and forcing it to be a pure string and having separate way of adding component. It is easier for now to leave it as is to avoid regressions.
Any thoughts on this?
I will update the PR in a bit with renaming the variables. Thanks
There was a problem hiding this comment.
this sounds good to me. if @matthewgallo has any questions or concerns i'll leave it up to him to post them here.
There was a problem hiding this comment.
Sounds like a plan for now, this way we don't interfere with any existing functionality (ie column headers already being able to be customized). 👍
There was a problem hiding this comment.
Thanks everyone, will leave a comment once I have the PR updated.
|
Updated the variable names, thanks! |
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheetBody.tsx
Outdated
Show resolved
Hide resolved
elycheea
added
the
status: needs design review 🎨
elycheea
changed the title
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheet.tsx
Outdated
Show resolved
Hide resolved
packages/ibm-products/src/components/DataSpreadsheet/DataSpreadsheet.tsx
Outdated
Show resolved
Hide resolved
|
Hey @AustinGitHub, one of the main pieces of design feedback is that the overflow menu should be square (32x32). |
|
Sure, I will take a look today |
|
Hi @matthewgallo since this is an optional component it is up to user to defined the width and height. I added the change but since this is optional component, it would be up to dev that provides component to add their own stylings to the component. But for storybook I put it to 32x32. Thanks! |
ee749e4
Closes #5253
What did you change?
Allow users to pass through their own custom component to the data spreadsheet table on column 0. So user can pass through icons, button, menu list, etc.
This component is also directional so user can specify
LeftorRightto determine which side they want the action to be.I also created new story for this feature.
How did you test and verify your work?
Storybook
Screen.Recording.2024-06-13.at.11.20.41.AM.mov
The width of the column 0 also scales with the number we put in to render so there shouldn't be any overflow issues.
Screen.Recording.2024-06-13.at.11.22.47.AM.mov