Skip to content

Commit 0fe6c52

Browse files
bennypowersbennypowers
authored
feat(core): internals controller improvements (#2296)
* feat(core): hook into formDisabledCallback in internalscontroller * feat(core): add ARIAMixin props to InternalsController * fix(core): rename InternalController#disabled to formDisabled * docs(button): demos for form control
1 parent f61bc8c commit 0fe6c52

File tree

12 files changed

+257
-16
lines changed

12 files changed

+257
-16
lines changed

‎.changeset/little-timers-repeat.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@patternfly/pfe-button": patch
3+
---
4+
5+
Allowed button to be disabled by its form without sprouting a disabled attr

‎.changeset/modern-readers-joke.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@patternfly/pfe-core": minor
3+
---
4+
5+
Added options to `InternalsController`. Use them to initialize `ARIA`
6+
properties.
7+
8+
```ts
9+
role: 'listbox',
10+
});
11+
```

‎.changeset/shy-knives-walk.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@patternfly/pfe-tools": patch
3+
---
4+
5+
Added constructible stylesheets polyfill to dev server

‎.changeset/spicy-pans-draw.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@patternfly/pfe-core": minor
3+
---
4+
5+
`InternalsController`: hook into host's `formDisabledCallback`

‎core/pfe-core/controllers/internals-controller.ts‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,85 @@
11
import type { ReactiveController, ReactiveControllerHost } from 'lit';
22

3-
export class InternalsController implements ReactiveController {
3+
function isARIAMixinProp(key: string): key is keyof ARIAMixin {
4+
return key === 'role' || key.startsWith('aria');
5+
}
6+
7+
export class InternalsController implements ReactiveController, ARIAMixin {
8+
declare role: ARIAMixin['role'];
9+
declare ariaAtomic: ARIAMixin['ariaAtomic'];
10+
declare ariaAutoComplete: ARIAMixin['ariaAutoComplete'];
11+
declare ariaBusy: ARIAMixin['ariaBusy'];
12+
declare ariaChecked: ARIAMixin['ariaChecked'];
13+
declare ariaColCount: ARIAMixin['ariaColCount'];
14+
declare ariaColIndex: ARIAMixin['ariaColIndex'];
15+
declare ariaColIndexText: ARIAMixin['ariaColIndexText'];
16+
declare ariaColSpan: ARIAMixin['ariaColSpan'];
17+
declare ariaCurrent: ARIAMixin['ariaCurrent'];
18+
declare ariaDisabled: ARIAMixin['ariaDisabled'];
19+
declare ariaExpanded: ARIAMixin['ariaExpanded'];
20+
declare ariaHasPopup: ARIAMixin['ariaHasPopup'];
21+
declare ariaHidden: ARIAMixin['ariaHidden'];
22+
declare ariaInvalid: ARIAMixin['ariaInvalid'];
23+
declare ariaKeyShortcuts: ARIAMixin['ariaKeyShortcuts'];
24+
declare ariaLabel: ARIAMixin['ariaLabel'];
25+
declare ariaLevel: ARIAMixin['ariaLevel'];
26+
declare ariaLive: ARIAMixin['ariaLive'];
27+
declare ariaModal: ARIAMixin['ariaModal'];
28+
declare ariaMultiLine: ARIAMixin['ariaMultiLine'];
29+
declare ariaMultiSelectable: ARIAMixin['ariaMultiSelectable'];
30+
declare ariaOrientation: ARIAMixin['ariaOrientation'];
31+
declare ariaPlaceholder: ARIAMixin['ariaPlaceholder'];
32+
declare ariaPosInSet: ARIAMixin['ariaPosInSet'];
33+
declare ariaPressed: ARIAMixin['ariaPressed'];
34+
declare ariaReadOnly: ARIAMixin['ariaReadOnly'];
35+
declare ariaRequired: ARIAMixin['ariaRequired'];
36+
declare ariaRoleDescription: ARIAMixin['ariaRoleDescription'];
37+
declare ariaRowCount: ARIAMixin['ariaRowCount'];
38+
declare ariaRowIndex: ARIAMixin['ariaRowIndex'];
39+
declare ariaRowIndexText: ARIAMixin['ariaRowIndexText'];
40+
declare ariaRowSpan: ARIAMixin['ariaRowSpan'];
41+
declare ariaSelected: ARIAMixin['ariaSelected'];
42+
declare ariaSetSize: ARIAMixin['ariaSetSize'];
43+
declare ariaSort: ARIAMixin['ariaSort'];
44+
declare ariaValueMax: ARIAMixin['ariaValueMax'];
45+
declare ariaValueMin: ARIAMixin['ariaValueMin'];
46+
declare ariaValueNow: ARIAMixin['ariaValueNow'];
47+
declare ariaValueText: ARIAMixin['ariaValueText'];
48+
449
#internals: ElementInternals;
550

51+
/** True when the control is disabled via it's containing fieldset element */
52+
get formDisabled() {
53+
return this.host.matches(':disabled');
54+
}
55+
56+
static protos = new WeakMap();
57+
658
constructor(
759
public host: ReactiveControllerHost & HTMLElement,
60+
options?: Partial<ARIAMixin>
861
) {
962
this.#internals = host.attachInternals();
63+
// proxy the internals object's aria prototype
64+
for (const key of Object.keys(Object.getPrototypeOf(this.#internals))) {
65+
if (isARIAMixinProp(key)) {
66+
Object.defineProperty(this, key, {
67+
get() {
68+
return this.#internals[key];
69+
},
70+
set(value) {
71+
this.#internals[key] = value;
72+
this.host.requestUpdate();
73+
}
74+
});
75+
}
76+
}
77+
78+
for (const [key, val] of Object.entries(options ?? {})) {
79+
if (isARIAMixinProp(key)) {
80+
this[key] = val;
81+
}
82+
}
1083
}
1184

1285
hostConnected?(): void

‎elements/pfe-button/BaseButton.ts‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { classMap } from 'lit/directives/class-map.js';
55
import { ifDefined } from 'lit/directives/if-defined.js';
66

77
import { InternalsController } from '@patternfly/pfe-core/controllers/internals-controller.js';
8-
98
import styles from './BaseButton.scss';
109

1110
/**
@@ -36,6 +35,9 @@ export abstract class BaseButton extends LitElement {
3635

3736
@property() name?: string;
3837

38+
/** Shorthand for the `icon` slot, the value is icon name */
39+
@property() icon?: string;
40+
3941
/** Changes the size of the button. */
4042
abstract size?: string;
4143

@@ -46,25 +48,12 @@ export abstract class BaseButton extends LitElement {
4648
*/
4749
abstract danger: unknown;
4850

49-
/** Shorthand for the `icon` slot, the value is icon name */
50-
@property() icon?: string;
51-
5251
#internals = new InternalsController(this);
5352

54-
#initiallyDisabled = this.hasAttribute('disabled');
55-
5653
protected get hasIcon() {
5754
return !!this.icon;
5855
}
5956

60-
formDisabledCallback(disabled: boolean) {
61-
this.disabled = disabled;
62-
}
63-
64-
formResetCallback() {
65-
this.disabled = this.#initiallyDisabled;
66-
}
67-
6857
override render() {
6958
const { hasIcon } = this;
7059
return html`
@@ -73,13 +62,18 @@ export abstract class BaseButton extends LitElement {
7362
value="${ifDefined(this.value)}"
7463
aria-label="${ifDefined(this.label)}"
7564
@click="${this.#onClick}"
76-
?disabled="${this.disabled}">
65+
?disabled="${this.disabled || this.#internals.formDisabled}">
7766
<slot id="icon" part="icon" aria-hidden="true" name="icon">${this.renderDefaultIcon()}</slot>
7867
<slot id="text" aria-hidden=${String(!!this.label) as 'true'|'false'}></slot>
7968
</button>
8069
`;
8170
}
8271

72+
protected async formDisabledCallback() {
73+
await this.updateComplete;
74+
this.requestUpdate();
75+
}
76+
8377
#onClick() {
8478
switch (this.type) {
8579
case 'reset':

‎elements/pfe-button/demo/form-control.html‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<script type="module" src="../form-control.js"></script>
2+
3+
<form>
4+
<h2>Nothing disabled</h2>
5+
<fieldset>
6+
<pfe-button>Ok</pfe-button>
7+
</fieldset>
8+
<label>Fieldset Disabled <pfe-switch name="fieldset"></pfe-switch> </label>
9+
<label>Button Disabled <pfe-switch name="button"></pfe-switch> </label>
10+
</form>
11+
12+
<form>
13+
<h2>fieldset disabled</h2>
14+
<fieldset disabled>
15+
<pfe-button>Ok</pfe-button>
16+
</fieldset>
17+
<label>Fieldset Disabled <pfe-switch name="fieldset" checked></pfe-switch> </label>
18+
<label>Button Disabled <pfe-switch name="button"></pfe-switch> </label>
19+
</form>
20+
21+
<form>
22+
<h2>Button disabled</h2>
23+
<fieldset>
24+
<pfe-button disabled>Ok</pfe-button>
25+
</fieldset>
26+
<label>Fieldset Disabled <pfe-switch name="fieldset"></pfe-switch> </label>
27+
<label>Button Disabled <pfe-switch name="button" checked></pfe-switch> </label>
28+
</form>
29+
30+
<form>
31+
<h2>Both disabled</h2>
32+
<fieldset disabled>
33+
<pfe-button disabled>Ok</pfe-button>
34+
</fieldset>
35+
<label>Fieldset Disabled <pfe-switch name="fieldset" checked></pfe-switch> </label>
36+
<label>Button Disabled <pfe-switch name="button" checked></pfe-switch> </label>
37+
</form>

‎elements/pfe-button/demo/form-control.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import '@patternfly/pfe-switch';
2+
import '@patternfly/pfe-button';
3+
4+
for (const form of document.querySelectorAll('form')) {
5+
form.addEventListener('submit', e=>e.preventDefault());
6+
form.addEventListener('change', sync);
7+
sync.call(form);
8+
}
9+
10+
/** @this{HTMLFormElement}*/
11+
function sync() {
12+
this.querySelector('fieldset').disabled = this.fieldset.checked;
13+
this.querySelector('pfe-button').disabled = this.button.checked;
14+
}

‎elements/pfe-button/test/pfe-button.spec.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect, html } from '@open-wc/testing';
22
import { createFixture } from '@patternfly/pfe-tools/test/create-fixture.js';
33
import { PfeButton } from '@patternfly/pfe-button';
4+
import { sendKeys } from '@web/test-runner-commands';
45

56
import '@patternfly/pfe-tools/test/stub-logger.js';
67

@@ -12,4 +13,86 @@ describe('<pfe-button>', function() {
1213
.and
1314
.to.be.an.instanceOf(PfeButton);
1415
});
16+
describe('in a fieldset', function() {
17+
let element: PfeButton;
18+
let fieldset: HTMLFieldSetElement;
19+
let form: HTMLFormElement;
20+
21+
beforeEach(async function() {
22+
form = await createFixture(html`
23+
<form>
24+
<input id="pre">
25+
<fieldset>
26+
<pfe-button>OK</pfe-button>
27+
</fieldset>
28+
<input id="post">
29+
</form>
30+
`);
31+
fieldset = form.querySelector('fieldset')!;
32+
element = form.querySelector('pfe-button')!;
33+
form.querySelector('input')?.focus();
34+
await element.updateComplete;
35+
});
36+
37+
describe('tabbing through', function() {
38+
beforeEach(async function() {
39+
await sendKeys({ press: 'Tab' });
40+
});
41+
it('does focus the button', function() {
42+
expect(document.activeElement)
43+
.to
44+
.be.an.instanceof(PfeButton);
45+
});
46+
});
47+
48+
describe('disabling the fieldset', function() {
49+
beforeEach(async function() {
50+
fieldset.disabled = true;
51+
await element.updateComplete;
52+
});
53+
it('disables the button', function() {
54+
expect(element.matches(':disabled'), 'matches :disabled').to.be.true;
55+
});
56+
describe('tabbing through', function() {
57+
beforeEach(async function() {
58+
await sendKeys({ press: 'Tab' });
59+
});
60+
it('does not focus the button', function() {
61+
expect(document.activeElement)
62+
.to
63+
.not
64+
.be.an.instanceof(PfeButton);
65+
});
66+
});
67+
// this was a regression spotted by @brianferry
68+
describe('then disabling the button', function() {
69+
beforeEach(async function() {
70+
element.disabled = true;
71+
await element.updateComplete;
72+
});
73+
describe('then enabling the button', function() {
74+
beforeEach(async function() {
75+
element.disabled = false;
76+
await element.updateComplete;
77+
});
78+
describe('then enabling the fieldset', function() {
79+
beforeEach(async function() {
80+
fieldset.disabled = false;
81+
await element.updateComplete;
82+
});
83+
describe('tabbing through', function() {
84+
beforeEach(async function() {
85+
await sendKeys({ press: 'Tab' });
86+
});
87+
it('does focus the button', function() {
88+
expect(document.activeElement)
89+
.to
90+
.be.an.instanceof(PfeButton);
91+
});
92+
});
93+
});
94+
});
95+
});
96+
});
97+
});
1598
});

‎package-lock.json‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)