fix(overflowmenu): added typing to align prop#19131
Conversation
|
Thanks for your submission! We ask that you all sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. 1 out of 2 committers have signed the DCO. |
|
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
2731b08 to
a9b073c
Compare
|
recheck |
tay1orjones
left a comment
There was a problem hiding this comment.
A couple comments but otherwise looks good to me. Thanks for taking this on!
Having align alongside direction and flipped is probably going to lead to some confusion. Prop/type comments help though since they explicitly mention "tooltip". The new OverflowMenu implementation calls it tooltipAlignment to avoid this confusion.
This being the old implementation though I don't think it's necessary to try and get a new tooltipAlignment to work alongside the existing align that some might be using. Plus it would add another layer of complexity having to iron out the types for all that.
I just want to make note of this, there's no action for you to take here.
* chore(overflowmenu): added todo waiting on consolidation of carbon-design-system#19081 * chore(overflowmenu-storybook): updated storybook align options to latest list
|
@jose-biescas The first commit a9b073c was made with a user/config that doesn't match your github.com profile, so the DCO bot will continually fail on this one. We can override it for this PR though, just something to be aware of next time. |
|
@tay1orjones Apologies for that, not sure what happened. Will make sure future commits are also verified/signed. Thank you! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19131 +/- ##
==========================================
- Coverage 84.37% 84.36% -0.02%
==========================================
Files 388 388
Lines 14547 14550 +3
Branches 4774 4753 -21
==========================================
+ Hits 12274 12275 +1
- Misses 2113 2116 +3
+ Partials 160 159 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…9131) * fix(overflowmenu): added typing to align prop * chore(overflowmenu): removed unnecessary types * chore(overflowmenu): added todo waiting on consolidation of carbon-design-system#19081 * chore(overflowmenu-storybook): updated storybook align options to latest list --------- Co-authored-by: jose-biescas <jose.biescas@ibm.com> Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
* fix(OverflowMenu): fix types Fix OverflowMenuProps to include properties that get passed through to IconButton, for example align, autoAlign, and tooltip. Relatedly, fix TableToolbarMenuProps to extend OverflowMenuProps. I also removed the aria-label as it's redundant with iconDescription. * chore(OverflowMenu): align is inherited from IconButtonProps PR #19131 added align manually but I changed it to inherit from IconButtonProps (rather than repeating the definition). --------- Co-authored-by: Riddhi Bansal <41935566+riddhybansal@users.noreply.github.com>
…9131) * fix(overflowmenu): added typing to align prop * chore(overflowmenu): removed unnecessary types * chore(overflowmenu): added todo waiting on consolidation of carbon-design-system#19081 * chore(overflowmenu-storybook): updated storybook align options to latest list --------- Co-authored-by: jose-biescas <jose.biescas@ibm.com> Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
* fix(OverflowMenu): fix types Fix OverflowMenuProps to include properties that get passed through to IconButton, for example align, autoAlign, and tooltip. Relatedly, fix TableToolbarMenuProps to extend OverflowMenuProps. I also removed the aria-label as it's redundant with iconDescription. * chore(OverflowMenu): align is inherited from IconButtonProps PR carbon-design-system#19131 added align manually but I changed it to inherit from IconButtonProps (rather than repeating the definition). --------- Co-authored-by: Riddhi Bansal <41935566+riddhybansal@users.noreply.github.com>
* fix(OverflowMenu): fix types Fix OverflowMenuProps to include properties that get passed through to IconButton, for example align, autoAlign, and tooltip. Relatedly, fix TableToolbarMenuProps to extend OverflowMenuProps. I also removed the aria-label as it's redundant with iconDescription. * chore(OverflowMenu): align is inherited from IconButtonProps PR carbon-design-system#19131 added align manually but I changed it to inherit from IconButtonProps (rather than repeating the definition). --------- Co-authored-by: Riddhi Bansal <41935566+riddhybansal@users.noreply.github.com>
Closes #18957
Adds a type to the
alignprop inOverflowMenuChangelog
New
alignpropertyTesting / Reviewing
Go to the
Defaultstorybook forOverflowMenuand thealignproperty can be controlled via a dropdown with allowed prop values