Skip to content

Commit

Permalink
fix(button): relate to this.href correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook Johnson authored and Westbrook committed Jan 5, 2021
1 parent d2d474e commit fade3ea
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 42 deletions.
50 changes: 37 additions & 13 deletions packages/button/src/ButtonBase.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ export class ButtonBase extends LikeAnchor(
return this.slotHasContent;
}

@query('.button')
private buttonElement!: HTMLButtonElement;
@query('.anchor')
private anchorElement!: HTMLButtonElement;

public get focusElement(): HTMLElement {
return this.buttonElement || this;
return this;
}

protected get buttonContent(): TemplateResult[] {
Expand All @@ -78,6 +78,14 @@ export class ButtonBase extends LikeAnchor(
return content;
}

constructor() {
super();

this.addEventListener('click', this.handleClickCapture, {
capture: true,
});
}

public click(): void {
if (this.disabled) {
return;
Expand All @@ -90,16 +98,29 @@ export class ButtonBase extends LikeAnchor(
super.click();
}

private handleClickCapture(event: Event): void | boolean {
if (this.disabled) {
event.preventDefault();
event.stopImmediatePropagation();
event.stopPropagation();
return false;
}
}

private shouldProxyClick(): boolean {
if (this.type !== 'button') {
let handled = false;
if (this.anchorElement) {
this.anchorElement.click();
handled = true;
} else if (this.type !== 'button') {
const proxy = document.createElement('button');
proxy.type = this.type;
this.insertAdjacentElement('afterend', proxy);
proxy.click();
proxy.remove();
return true;
handled = true;
}
return false;
return handled;
}

protected renderButton(): TemplateResult {
Expand All @@ -112,7 +133,7 @@ export class ButtonBase extends LikeAnchor(
return this.href && this.href.length > 0
? this.renderAnchor({
id: 'button',
className: 'button',
className: 'button anchor',
anchorContent: this.buttonContent,
})
: this.renderButton();
Expand All @@ -122,8 +143,10 @@ export class ButtonBase extends LikeAnchor(
const { code } = event;
switch (code) {
case 'Space':
this.addEventListener('keyup', this.handleKeyup);
this.active = true;
if (typeof this.href === 'undefined') {
this.addEventListener('keyup', this.handleKeyup);
this.active = true;
}
break;
default:
break;
Expand Down Expand Up @@ -161,12 +184,8 @@ export class ButtonBase extends LikeAnchor(
private manageRole(): void {
if (this.href && this.href.length > 0) {
this.removeAttribute('role');
this.removeEventListener('keydown', this.handleKeydown);
this.removeEventListener('keypress', this.handleKeypress);
} else if (!this.hasAttribute('role')) {
this.setAttribute('role', 'button');
this.addEventListener('keydown', this.handleKeydown);
this.addEventListener('keypress', this.handleKeypress);
}
}

Expand All @@ -177,6 +196,8 @@ export class ButtonBase extends LikeAnchor(
}
this.manageRole();
this.addEventListener('click', this.shouldProxyClick);
this.addEventListener('keydown', this.handleKeydown);
this.addEventListener('keypress', this.handleKeypress);
}

protected updated(changed: PropertyValues): void {
Expand All @@ -194,5 +215,8 @@ export class ButtonBase extends LikeAnchor(
this.removeEventListener('focusout', this.handleFocusout);
}
}
if (this.anchorElement) {
this.anchorElement.tabIndex = -1;
}
}
}
9 changes: 9 additions & 0 deletions packages/button/src/button-base.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ governing permissions and limitations under the License.
vertical-align: top;
}

:host([disabled]) {
pointer-events: none;
cursor: auto;
}

#button {
color: inherit;
text-decoration: inherit;
Expand All @@ -27,6 +32,10 @@ governing permissions and limitations under the License.
outline: none;
}

:host:after {
pointer-events: none;
}

slot[name='icon']::slotted(:not(sp-icon)) {
fill: currentColor;
stroke: currentColor;
Expand Down
70 changes: 61 additions & 9 deletions packages/button/test/button.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Button', () => {
it('loads default', async () => {
const el = await fixture<Button>(
html`
<sp-button>Button</sp-button>
<sp-button tabindex="0">Button</sp-button>
`
);

Expand All @@ -53,7 +53,7 @@ describe('Button', () => {
it('loads default w/ an icon', async () => {
const el = await fixture<Button>(
html`
<sp-button>
<sp-button label="">
Button
<svg slot="icon"></svg>
</sp-button>
Expand All @@ -63,6 +63,8 @@ describe('Button', () => {
await elementUpdated(el);
expect(el).to.not.be.undefined;
expect(el.textContent).to.include('Button');
expect(!((el as unknown) as { hasIcon: boolean }).hasIcon);
expect(!((el as unknown) as { iconRight: boolean }).iconRight);
await expect(el).to.be.accessible();
});
it('loads default w/ an icon-right', async () => {
Expand Down Expand Up @@ -191,28 +193,38 @@ describe('Button', () => {
expect(focusedCount).to.equal(1);
});
it('manages `disabled`', async () => {
const clickSpy = spy();
const el = await fixture<Button>(
html`
<sp-button>
<sp-button @click=${() => clickSpy()}>
Button
</sp-button>
`
);

await elementUpdated(el);
const focusElement = el.focusElement as HTMLButtonElement;

expect(focusElement.disabled, 'initially not').to.be.false;
el.click();
await elementUpdated(el);
expect(clickSpy.calledOnce);

clickSpy.resetHistory();
el.disabled = true;
await elementUpdated(el);
el.click();
await elementUpdated(el);
expect(clickSpy.callCount).to.equal(0);

expect(focusElement.disabled).to.be.true;
clickSpy.resetHistory();
await elementUpdated(el);
el.dispatchEvent(new Event('click', {}));
await elementUpdated(el);
expect(clickSpy.callCount).to.equal(0);

clickSpy.resetHistory();
el.disabled = false;
el.click();
await elementUpdated(el);

expect(focusElement.disabled, 'finally not').to.be.false;
expect(clickSpy.calledOnce);
});
it('manages `aria-disabled`', async () => {
const el = await fixture<Button>(
Expand Down Expand Up @@ -445,4 +457,44 @@ describe('Button', () => {
expect(submitSpy.callCount).to.equal(1);
expect(resetSpy.callCount).to.equal(1);
});
it('proxies click by [href]', async () => {
const clickSpy = spy();
const el = await fixture<Button>(
html`
<sp-button href="test_url">With Href</sp-button>
`
);

await elementUpdated(el);
((el as unknown) as {
anchorElement: HTMLAnchorElement;
}).anchorElement.addEventListener('click', (event: Event): void => {
event.preventDefault();
event.stopPropagation();
clickSpy();
});
expect(clickSpy.callCount).to.equal(0);

el.click();
await elementUpdated(el);
expect(clickSpy.callCount).to.equal(1);
});
it('manages "active" while focused', async () => {
const el = await fixture<Button>(
html`
<sp-button label="Button">
<svg slot="icon"></svg>
</sp-button>
`
);

await elementUpdated(el);
el.active = true;
await elementUpdated(el);

el.dispatchEvent(new FocusEvent('focusout'));
await elementUpdated(el);

expect(el.active).to.be.false;
});
});
43 changes: 28 additions & 15 deletions packages/shared/src/focusable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ export class Focusable extends FocusVisiblePolyfillMixin(SpectrumElement) {
@property({ type: Number })
public get tabIndex(): number {
if (this.focusElement === this) {
return Number(this.getAttribute('tabindex')) || -1;
const tabindex = this.hasAttribute('tabindex')
? Number(this.getAttribute('tabindex'))
: NaN;
return !isNaN(tabindex) ? tabindex : -1;
}
const tabIndexAttribute = parseFloat(
this.hasAttribute('tabindex')
Expand All @@ -59,26 +62,28 @@ export class Focusable extends FocusVisiblePolyfillMixin(SpectrumElement) {
}
// When `focusElement` isn't available yet,
// use host tabindex as the cache.
if (!this.focusElement || this.focusElement.isSameNode(this)) {
if (!this.focusElement) {
return tabIndexAttribute;
}
// All other times, use the tabindex of `focusElement`
// as the cache for this value.
return this.focusElement.tabIndex;
}
public set tabIndex(tabIndex: number) {
if (this.focusElement === this) {
if (tabIndex !== this.tabIndex) {
this.setAttribute('tabindex', '' + tabIndex);
}
return;
}
// Flipping `manipulatingTabindex` to true before a change
// allows for that change NOT to effect the cached value of tabindex
if (this.manipulatingTabindex) {
this.manipulatingTabindex = false;
return;
}
if (this.focusElement === this) {
if (tabIndex !== this.tabIndex) {
this._tabIndex = tabIndex;
const tabindex = this.disabled ? '-1' : '' + tabIndex;
this.setAttribute('tabindex', tabindex);
}
return;
}
// All code paths are about to address the host tabindex without side effect.
this.manipulatingTabindex = true;
if (tabIndex === -1 || this.disabled) {
Expand All @@ -102,6 +107,7 @@ export class Focusable extends FocusVisiblePolyfillMixin(SpectrumElement) {
}
this.manageFocusElementTabindex(tabIndex);
}
private _tabIndex = 0;

private async manageFocusElementTabindex(tabIndex: number): Promise<void> {
if (!this.focusElement) {
Expand Down Expand Up @@ -199,23 +205,30 @@ export class Focusable extends FocusVisiblePolyfillMixin(SpectrumElement) {
disabled: boolean,
oldDisabled: boolean
): Promise<void> {
const canSetDisabled = (): boolean =>
this.focusElement !== this &&
typeof this.focusElement.disabled !== 'undefined';
if (disabled) {
this.manipulatingTabindex = true;
this.setAttribute('tabindex', '-1');
await this.updateComplete;
if (typeof this.focusElement.disabled === 'undefined') {
this.setAttribute('aria-disabled', 'true');
} else {
if (canSetDisabled()) {
this.focusElement.disabled = true;
} else {
this.setAttribute('aria-disabled', 'true');
}
} else if (oldDisabled) {
this.manipulatingTabindex = true;
this.removeAttribute('tabindex');
await this.updateComplete;
if (typeof this.focusElement.disabled === 'undefined') {
this.removeAttribute('aria-disabled');
if (this.focusElement === this) {
this.setAttribute('tabindex', '' + this._tabIndex);
} else {
this.removeAttribute('tabindex');
}
await this.updateComplete;
if (canSetDisabled()) {
this.focusElement.disabled = false;
} else {
this.removeAttribute('aria-disabled');
}
}
}
Expand Down
Loading

0 comments on commit fade3ea

Please sign in to comment.