add d2l-navigation-button-icon component #151
Conversation
|
Visual diff tests failed - pull request #152 has been opened with the updated goldens. |
dlockhart
force-pushed
the
dlockhart/button-icon
branch
from
f140563
to
95ec27b
Compare
| @@ -253,11 +268,11 @@ Then run the tests: | |||
|
|
|||
| ```shell | |||
| # run visual-diff tests | |||
| npx mocha './test/**/*.visual-diff.mjs' -t 10000 | |||
| npx mocha './test/**/*.visual-diff.js' -t 10000 | |||
There was a problem hiding this comment.
Missed this last time around.
| button[disabled] { | ||
| cursor: default; | ||
| opacity: 0.5; | ||
| } |
There was a problem hiding this comment.
These styles are ported over from the d2l-navigation-shared-styles.js Polymer style mixins, but then heavily modified since there's no <slot>, no IE11/Edge hacks and the "highlight border" styles are already in a shared export.
| const text = !this.textHidden ? this.text : nothing; | ||
| const tooltip = this.textHidden ? html`<d2l-tooltip for="${this._buttonId}" for-type="label">${this.text}</d2l-tooltip>` : nothing; | ||
| return html` | ||
| <button id="${ifDefined(id)}" ?disabled="${this.disabled}" aria-label="${ifDefined(ariaLabel)}"> |
There was a problem hiding this comment.
I'm setting aria-label AND associating the tooltip because of this defect.
| import '../d2l-navigation-button-notification-icon.js'; | ||
| import '../d2l-navigation-link.js'; | ||
| import '../d2l-navigation-link-back.js'; | ||
| import '../d2l-navigation-link-image.js'; |
There was a problem hiding this comment.
I split the link tests out into link.test.js to match what the visual-diffs were doing.
| <div class="visual-diff"> | ||
| <d2l-navigation-button-icon id="icon-text-disabled" icon="tier3:classes" text="Classes" disabled></d2l-navigation-button-icon> | ||
| </div> | ||
| <div class="visual-diff" id="icon-text-hidden-container" style="display: inline-block; padding: 0 30px 60px 30px;"> |
There was a problem hiding this comment.
Padding out the parent so that the tooltip can fit nicely inside the space it creates.
Co-authored-by: github-actions <[email protected]>
|
🎉 This PR is included in version 5.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
ghost
added
the
A new component that mirrors the functionality of the recently added
<d2l-navigation-link-icon>, except with button semantics. Loosely based on the more generic<d2l-navigation-button>that takes a<slot>. I'm hoping to switch all usages of<d2l-navigation-button>over to this and get rid of it.When the text is hidden, I have it rendering a tooltip instead of using the
title-- this matches what the iterators are doing and will allow me to just switch them over to use this. I've updated link to do this as well, which isn't a breaking change since the only usages of them don't hide the text (FYI @abcotter).