feat(ui-shell): support button tooltip in HeaderGlobalAction
#1894
Conversation
| ref?: null | HTMLButtonElement; | ||
|
|
||
| [key: `data-${string}`]: any; | ||
| ref?: undefined; |
There was a problem hiding this comment.
I am not sure why it was updated this way by yarn build:docs. Should I edit this file manually?
There was a problem hiding this comment.
The binding probably should be updated if the button element should be exposed.
Right now the component instance is bound, i.e. change this:
- <Button bind:this="{ref}" class="{buttonClass}" {...$$restProps} on:click>
+ <Button bind:ref class="{buttonClass}" {...$$restProps} on:click>Don't know if that would reinstate the type inference, though; if not, it should be possible to use @type in the JSDoc of the property in the component. The generated files should not be touched.
There was a problem hiding this comment.
I applied this suggestion, but the .d.ts file did not update properly until I specified the type in JSDoc explicitly
spburtsev
changed the title
| <svelte:component this="{icon}" size="{20}" /> | ||
| </slot> | ||
| </button> | ||
| <Button bind:this="{ref}" class="{buttonClass}" {...$$restProps} on:click> |
There was a problem hiding this comment.
The order of the properties matters and probably causes issues.
class: directives work independently from a class property passed in via $$restProps.
But now that a component is used, I suspect that when a class is set, the spread will overwrite what was set before to buttonClass.
If that is the case, the spread should happen before setting class or the component should define a class property, so it no longer is part of $$restProps.
There was a problem hiding this comment.
I did not find any differences depending on the order of these props, but just in case I put $$restProps before class.
metonym
force-pushed
the
use-carbon-button-for-header-action-global
branch
from
e6e922f to
ce3dd9f
Compare
metonym
changed the title
There was a problem hiding this comment.
I appreciate the both of you for working on this! Tried it myself and it looks great.
I made a couple of changes:
- 6ef0f06: Also apply the refactored icon slot logic to the link button variant (I noticed locally that the icon failed to render for link buttons)
- 321813f: Tweak the Header Utilities example to include three
HeaderGlobalActioncomponents to demo and test out the various tooltip positions
I'll get this out in the next minor version release.
metonym
changed the title
HeaderGlobalAction
Breaking change : Use Button component in HeaderGlobalAction
** Issue: #1893
** Edit: an example of updated HeaderGlobalAction