Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add d2l-navigation-button-icon component #151

Merged
merged 5 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,21 @@ Add the `d2l-navigation-main-footer` component, and provide elements for the `ma

---

### d2l-navigation-button-icon

`<d2l-navigation-button-icon>` provides a button with an icon and optional text.

### Properties

| Property | Type | Description |
|--|--|--|
| `disabled` | Boolean | Disables the button |
| `text` | String, required | Text for the button |
| `icon` | String | Preset icon key (e.g. `tier1:gear`) |
| `text-hidden` | Boolean | Visually hides the text |

---

### d2l-navigation-link-icon

Similar to `<d2l-navigation-button-icon>`, a link that comes with an icon and optional text.
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this last time around.

# subset of visual-diff tests:
npx mocha './test/**/*.visual-diff.mjs' -t 10000 -g some-pattern
npx mocha './test/**/*.visual-diff.js' -t 10000 -g some-pattern
# update visual-diff goldens
npx mocha './test/**/*.visual-diff.mjs' -t 10000 --golden
npx mocha './test/**/*.visual-diff.js' -t 10000 --golden
```

### Running the demos
Expand Down
126 changes: 126 additions & 0 deletions d2l-navigation-button-icon.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import '@brightspace-ui/core/components/colors/colors.js';
import '@brightspace-ui/core/components/icons/icon.js';
import '@brightspace-ui/core/components/tooltip/tooltip.js';
import { css, html, LitElement, nothing } from 'lit';
import { FocusMixin } from '@brightspace-ui/core/mixins/focus-mixin.js';
import { getUniqueId } from '@brightspace-ui/core/helpers/uniqueId.js';
import { ifDefined } from 'lit/directives/if-defined.js';
import { highlightBorderStyles } from './d2l-navigation-styles.js';

/**
* Navigation button with an icon and text.
*/
class NavigationButtonIcon extends FocusMixin(LitElement) {

static get properties() {
return {
/**
* Disables the button
* @type {boolean}
*/
disabled: { reflect: true, type: Boolean },
/**
* REQUIRED: Preset icon key (e.g. "tier1:gear")
* @type {string}
*/
icon: { type: String },
/**
* REQUIRED: Text for the button
* @type {string}
*/
text: { type: String },
/**
* Visually hides the text but still accessible
* @type {boolean}
*/
textHidden: { attribute: 'text-hidden', type: Boolean }
};
}

static get styles() {
return [highlightBorderStyles, css`
:host {
display: inline-block;
height: 100%;
}
:host([hidden]) {
display: none;
}
button {
align-items: center;
background: transparent;
border: none;
color: var(--d2l-color-ferrite);
cursor: pointer;
font-family: inherit;
font-size: inherit;
height: 100%;
margin: 0;
min-height: 40px;
outline: none;
overflow: visible;
padding: 0;
position: relative;
white-space: nowrap;
}
/* Firefox includes a hidden border which messes up button dimensions */
button::-moz-focus-inner {
border: 0;
}
button:not([disabled]):hover,
button:not([disabled]):focus {
--d2l-icon-fill-color: var(--d2l-color-celestine);
color: var(--d2l-color-celestine);
outline: none;
}
button[disabled] {
cursor: default;
opacity: 0.5;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

`];
}

constructor() {
super();
this.disabled = false;
this.textHidden = false;
this._buttonId = getUniqueId();
}

static get focusElementSelector() {
return 'button';
}

render() {
const { ariaLabel, id, text, tooltip } = this._getRenderSettings();
const highlightBorder = !this.disabled ? html`<span class="d2l-navigation-highlight-border"></span>` : nothing;
return html`
<button id="${ifDefined(id)}" ?disabled="${this.disabled}" aria-label="${ifDefined(ariaLabel)}">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm setting aria-label AND associating the tooltip because of this defect.

${highlightBorder}
<d2l-icon icon="${this.icon}"></d2l-icon>
${text}
</button>
${tooltip}
`;
}

_getRenderSettings() {
if (this.textHidden) {
return {
ariaLabel: this.text,
id: this._buttonId,
text: nothing,
tooltip: html`<d2l-tooltip for="${this._buttonId}" for-type="label">${this.text}</d2l-tooltip>`
};
}
return {
ariaLabel: undefined,
id: undefined,
text: this.text,
tooltip: nothing
};
}

}

customElements.define('d2l-navigation-button-icon', NavigationButtonIcon);
34 changes: 26 additions & 8 deletions d2l-navigation-link-icon.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import '@brightspace-ui/core/components/icons/icon.js';
import { html, LitElement } from 'lit';
import { classMap } from 'lit/directives/class-map.js';
import '@brightspace-ui/core/components/tooltip/tooltip.js';
import { html, LitElement, nothing } from 'lit';
import { FocusMixin } from '@brightspace-ui/core/mixins/focus-mixin.js';
import { getUniqueId } from '@brightspace-ui/core/helpers/uniqueId.js';
import { ifDefined } from 'lit/directives/if-defined.js';
import { offscreenStyles } from '@brightspace-ui/core/components/offscreen/offscreen.js';
import { highlightBorderStyles, highlightLinkStyles } from './d2l-navigation-styles.js';
Expand All @@ -24,7 +25,7 @@ class NavigationLinkIcon extends FocusMixin(LitElement) {
*/
icon: { type: String },
/**
* REQUIRED: Accessible text for the button
* REQUIRED: Text for the link
* @type {string}
*/
text: { type: String },
Expand All @@ -43,6 +44,7 @@ class NavigationLinkIcon extends FocusMixin(LitElement) {
constructor() {
super();
this.textHidden = false;
this._linkId = getUniqueId();
this._missingHrefErrorHasBeenThrown = false;
this._validatingHrefTimeout = null;
}
Expand All @@ -57,15 +59,14 @@ class NavigationLinkIcon extends FocusMixin(LitElement) {
}

render() {
const textClasses = {
'd2l-offscreen': this.textHidden
};
const { ariaLabel, id, text, tooltip } = this._getRenderSettings();
return html`
<a href="${ifDefined(this.href)}" title="${ifDefined(this.textHidden ? this.text : undefined)}">
<a id="${ifDefined(id)}" href="${ifDefined(this.href)}" aria-label="${ifDefined(ariaLabel)}">
<span class="d2l-navigation-highlight-border"></span>
<d2l-icon icon="${this.icon}"></d2l-icon>
<span class="${classMap(textClasses)}">${this.text}</span>
${text}
</a>
${tooltip}
`;
}

Expand All @@ -77,6 +78,23 @@ class NavigationLinkIcon extends FocusMixin(LitElement) {

}

_getRenderSettings() {
if (this.textHidden) {
return {
ariaLabel: this.text,
id: this._linkId,
text: nothing,
tooltip: html`<d2l-tooltip for="${this._linkId}" for-type="label">${this.text}</d2l-tooltip>`
};
}
return {
ariaLabel: undefined,
id: undefined,
text: this.text,
tooltip: nothing
};
}

_validateHref() {
clearTimeout(this._validatingHrefTimeout);
// don't error immediately in case it doesn't get set immediately
Expand Down
10 changes: 7 additions & 3 deletions demo/button-link.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import '@brightspace-ui/core/components/icons/icon.js';
import '../d2l-navigation-button.js';
import '../d2l-navigation-button-close.js';
import '../d2l-navigation-button-icon.js';
import '../d2l-navigation-button-notification-icon.js';
import '../d2l-navigation-link.js';
import '../d2l-navigation-link-back.js';
Expand Down Expand Up @@ -44,9 +45,11 @@ <h2>d2l-navigation-button / d2l-navigation-link</h2>
</div>
</d2l-demo-snippet>

<h2>d2l-navigation-link-icon</h2>
<h2>d2l-navigation-button-icon / d2l-navigation-link-icon</h2>
<d2l-demo-snippet>
<div class="wrapper">
<d2l-navigation-button-icon icon="tier3:gear" text="Settings"></d2l-navigation-button-icon>
<d2l-navigation-button-icon icon="tier3:gear" text="Settings (hidden)" text-hidden></d2l-navigation-button-icon>
<d2l-navigation-link-icon href="https://www.example.org" icon="tier3:gear" text="Settings"></d2l-navigation-link-icon>
<d2l-navigation-link-icon href="https://www.example.org" icon="tier3:gear" text="Settings (hidden)" text-hidden></d2l-navigation-link-icon>
</div>
Expand Down Expand Up @@ -93,13 +96,14 @@ <h2>Combined</h2>
href="https://www.example.org"
src="./logo-image.png"
text="Link Image Text"></d2l-navigation-link-image>
<d2l-navigation-button-icon icon="tier3:gear" text="Settings"></d2l-navigation-button-icon>
<d2l-navigation-link-icon href="https://www.example.org" icon="tier3:gear" text="Settings"></d2l-navigation-link-icon>
<d2l-navigation-link href="https://www.example.org">
<d2l-icon icon="tier2:gear" style="margin-right: 10px;"></d2l-icon>
<d2l-icon icon="tier3:gear" style="margin-right: 10px;"></d2l-icon>
Settings
</d2l-navigation-link>
<d2l-navigation-button>
<d2l-icon icon="tier2:gear" style="margin-right: 10px;"></d2l-icon>
<d2l-icon icon="tier3:gear" style="margin-right: 10px;"></d2l-icon>
Settings
</d2l-navigation-button>
<d2l-navigation-button-notification-icon notification icon="tier3:notification-bell" text="Notification on" id="notified" notification-text="You have new notifications"></d2l-navigation-button-notification-icon>
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"/lang",
"d2l-navigation-band.js",
"d2l-navigation-button-close.js",
"d2l-navigation-button-icon.js",
"d2l-navigation-button-notification-icon.js",
"d2l-navigation-button.js",
"d2l-navigation-highlight-styles.js",
Expand Down
58 changes: 21 additions & 37 deletions test/button.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import '../d2l-navigation-button.js';
import '../d2l-navigation-button-close.js';
import '../d2l-navigation-button-icon.js';
import '../d2l-navigation-button-notification-icon.js';
import '../d2l-navigation-link.js';
import '../d2l-navigation-link-back.js';
import '../d2l-navigation-link-image.js';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split the link tests out into link.test.js to match what the visual-diffs were doing.

import { expect, fixture, html, oneEvent } from '@open-wc/testing';
import { getComposedActiveElement } from '@brightspace-ui/core/helpers/focus.js';
import { runConstructor } from '@brightspace-ui/core/tools/constructor-test-helper.js';

describe('Buttons', () => {
Expand Down Expand Up @@ -74,61 +73,46 @@ describe('Buttons', () => {

});

describe('d2l-navigation-link', () => {
describe('d2l-navigation-button-icon', () => {

describe('accessibility', () => {

it('default', async() => {
const el = await fixture(html`<d2l-navigation-link href="https://www.d2l.com" text="D2L">D2L</d2l-navigation-link>`);
const el = await fixture(html`<d2l-navigation-button-icon icon="tier3:gear" text="test"></d2l-navigation-button-icon>`);
await expect(el).to.be.accessible();
});

it('focused', async() => {
const el = await fixture(html`<d2l-navigation-link href="https://www.d2l.com" text="D2L">D2L</d2l-navigation-link>`);
setTimeout(() => el.shadowRoot.querySelector('a').focus());
await oneEvent(el, 'focus');
it('text hidden', async() => {
const el = await fixture(html`<d2l-navigation-button-icon icon="tier3:gear" text="test" text-hidden></d2l-navigation-button-icon>`);
await expect(el).to.be.accessible();
});

});

describe('constructor', () => {
it('should construct', () => {
runConstructor('d2l-navigation-link');
it('disabled', async() => {
const el = await fixture(html`<d2l-navigation-button-icon icon="tier3:gear" text="test" disabled></d2l-navigation-button-icon>`);
await expect(el).to.be.accessible();
});
});

});

describe('d2l-navigation-link-back', () => {

describe('accessibility', () => {
it('should pass all aXe tests', async() => {
const el = await fixture(html`<d2l-navigation-link-back href="https://www.d2l.com" text="D2L">D2L</d2l-navigation-link-back>`);
it('focused', async() => {
const el = await fixture(html`<d2l-navigation-button-icon icon="tier3:gear" text="test"></d2l-navigation-button-icon>`);
el.focus();
const activeElem = getComposedActiveElement();
expect(activeElem).to.equal(el.shadowRoot.querySelector('button'));
await expect(el).to.be.accessible();
});

});

describe('constructor', () => {
it('should construct', () => {
runConstructor('d2l-navigation-link-back');
runConstructor('d2l-navigation-button-icon');
});
});

});

describe('d2l-navigation-link-image', () => {

describe('accessibility', () => {
it('should pass all aXe tests', async() => {
const el = await fixture(html`<d2l-navigation-link-image src="../demo/logo-image.png" href="https:/www.d2l.com" text="D2L"></d2l-navigation-link-image>`);
await expect(el).to.be.accessible();
});
});

describe('constructor', () => {
it('should construct', () => {
runConstructor('d2l-navigation-link-image');
describe('events', () => {
it('should trigger click event', async() => {
const el = await fixture(html`<d2l-navigation-button-icon icon="tier3:gear" text="test"></d2l-navigation-button-icon>`);
setTimeout(() => el.shadowRoot.querySelector('button').click());
await oneEvent(el, 'click');
});
});

Expand Down
Loading