fix(DataSpreadsheet): overflow menus in row headers #7448
Conversation
jlongshore
requested review from
anamikaanu96 and
elycheea
and removed request for
a team
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7448 +/- ##
==========================================
- Coverage 82.47% 81.63% -0.84%
==========================================
Files 324 411 +87
Lines 10779 18937 +8158
Branches 3543 4223 +680
==========================================
+ Hits 8890 15460 +6570
- Misses 1889 3477 +1588
🚀 New features to boost your workflow:
|
matthewgallo
left a comment
matthewgallo
left a comment
There was a problem hiding this comment.
Looks good, just notice some slight visual differences in the layout of the row header cells
With current changes
With previous changes
matthewgallo
left a comment
matthewgallo
left a comment
There was a problem hiding this comment.
Approving after chatting with @jlongshore a bit more about the specs for spreadsheet row headers 🎉
elycheea
changed the title
elycheea
changed the title
elycheea
changed the title
|
@jlongshore looks like there’s a conflict in the snapshots. You might just need to run it again — I think it’s just |
| onClick={handleRowHeaderClickEvent(index)} | ||
| className={cx( | ||
| `${blockClass}__td`, | ||
| // `${blockClass}__td`, |
There was a problem hiding this comment.
minor, but removing would be preferred to commenting out unless there's like a really good reason to leave that there
elycheea
added
the
needs: design opinion
|
Also tagging this as
needs: design opinion
Note that clicking on the row number does select/highlight the full row. |
davidmenendez
left a comment
davidmenendez
left a comment
There was a problem hiding this comment.
looks good to me, but i believe we're still waiting for a design opinion. @RichKummer whenever you have some time to look 👍
4 participants
Closes #7180
Issue was when adding an overflow menu to the row header, we would end up with nested buttons. This effort was to make those elements siblings rather than parent/child...
What did you change?
Moved the rendering of other components in row headers to be siblings to the header button. Had to shell game some styles.
How did you test and verify your work?
Storybook, test script