Skip to content

Commit 8b96f26

Browse files
authored
update AsyncButton to fix problem with keyboard nav focus (#13954)
* update AsyncButton to fix problem with not being able to have to keyboard nav focus back to the element when performing an action triggered by AsyncButton * fix e2e test * fix e2e test
1 parent 522109b commit 8b96f26

File tree

5 files changed

+44
-17
lines changed

5 files changed

+44
-17
lines changed

cypress/e2e/po/components/async-button.po.ts

+4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ export default class AsyncButtonPo extends ComponentPo {
1313
return this.self().should('not.have.attr', 'disabled');
1414
}
1515

16+
waitForDisabledAppearanceToDisappear(): Cypress.Chainable {
17+
return this.self().should('have.class', 'ready-for-action');
18+
}
19+
1620
label(name: string): Cypress.Chainable {
1721
return this.self().contains(name);
1822
}

cypress/e2e/tests/pages/global-settings/branding.spec.ts

+13-8
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ const settings = {
1717
new: 'Rancher e2e'
1818
},
1919
primaryColor: {
20-
original: '#3d98d3', // 3D98D3
21-
new: '#f80dd8',
22-
newRGB: 'rgb(248, 13, 216)', // 'rgb(220, 222, 231)'
20+
original: '#3d98d3', // 3D98D3
21+
new: '#f80dd8',
22+
newRGB: 'rgb(248, 13, 216)', // 'rgb(220, 222, 231)'
23+
// the browser seems to sometimes slightly change the color
24+
// don't know if it's related to the actual color input output
25+
// OR the application of the color in the css
26+
// check PR https://github.com/rancher/dashboard/pull/13954 description
27+
validNewRGBs: ['rgb(248, 13, 216)', 'rgb(249, 63, 224)']
2328
},
2429
linkColor: {
2530
original: '#3d98d3', // #3D98D3
@@ -352,39 +357,39 @@ describe('Branding', { testIsolation: 'off' }, () => {
352357

353358
BrandingPagePo.navTo();
354359

355-
// Set
356360
brandingPage.primaryColorCheckbox().set();
357361
brandingPage.primaryColorPicker().value().should('not.eq', settings.primaryColor.new);
358362
brandingPage.primaryColorPicker().set(settings.primaryColor.new);
359363
brandingPage.applyAndWait('**/ui-primary-color', 200);
364+
brandingPage.applyButton().waitForDisabledAppearanceToDisappear();
360365

361366
// Check in session
362367
brandingPage.primaryColorPicker().value().should('eq', settings.primaryColor.new);
363368
brandingPage.primaryColorPicker().previewColor().should('eq', settings.primaryColor.newRGB);
364369
brandingPage.applyButton().self().should('have.css', 'background').should((background: string) => {
365-
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.newRGB));
370+
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.validNewRGBs[0]) || b.startsWith(settings.primaryColor.validNewRGBs[1]));
366371
});
367372

368373
// Check over reload
369374
cy.reload();
370375
brandingPage.primaryColorPicker().value().should('eq', settings.primaryColor.new);
371376
brandingPage.primaryColorPicker().previewColor().should('eq', settings.primaryColor.newRGB);
372377
brandingPage.applyButton().self().should('have.css', 'background').should((background: string) => {
373-
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.newRGB));
378+
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.validNewRGBs[0]) || b.startsWith(settings.primaryColor.validNewRGBs[1]));
374379
});
375380

376381
// check that login page has new styles applied
377382
// https://github.com/rancher/dashboard/issues/10788
378383
loginPage.goTo();
379384

380385
loginPage.submitButton().self().should('have.css', 'background').should((background: string) => {
381-
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.newRGB));
386+
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.validNewRGBs[0]) || b.startsWith(settings.primaryColor.validNewRGBs[1]));
382387
});
383388

384389
cy.reload();
385390

386391
loginPage.submitButton().self().should('have.css', 'background').should((background: string) => {
387-
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.newRGB));
392+
expect(background).to.satisfy((b) => b.startsWith(settings.primaryColor.validNewRGBs[0]) || b.startsWith(settings.primaryColor.validNewRGBs[1]));
388393
});
389394
// EO test https://github.com/rancher/dashboard/issues/10788
390395

shell/assets/styles/global/_button.scss

+1
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ fieldset[disabled] .btn {
215215
}
216216
}
217217

218+
.btn-disabled,
218219
.btn[disabled] {
219220
// Ensure disabled button's border remains as-is, otherwise button appears vertically shorter than others in group
220221
border: inherit;

shell/components/AsyncButton.vue

+24-7
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export const ASYNC_BUTTON_STATES = {
1111
1212
const TEXT = 'text';
1313
const TOOLTIP = 'tooltip';
14+
const DISABLED_CLASS_STYLE = 'btn-disabled';
1415
1516
export type AsyncButtonCallback = (success: boolean) => void;
1617
@@ -152,9 +153,29 @@ export default defineComponent({
152153
out[`btn-${ this.size }`] = true;
153154
}
154155
156+
// while we are waiting for the async button to get
157+
// it's callback we want to the button to appear as disabled
158+
// but not being actually disabled as need it to be
159+
// able to return the keyboard navigation focus back to it
160+
// which can't be done while actually disabled, as per
161+
// https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols
162+
if (this.phase === ASYNC_BUTTON_STATES.WAITING) {
163+
out[DISABLED_CLASS_STYLE] = true;
164+
}
165+
166+
// used to assist e2e testing mostly when waiting for button to return
167+
// to it's normal state/phase
168+
if (this.phase === ASYNC_BUTTON_STATES.ACTION) {
169+
out['ready-for-action'] = true;
170+
}
171+
155172
return out;
156173
},
157174
175+
appearsDisabled(): boolean {
176+
return this.disabled || this.phase === ASYNC_BUTTON_STATES.WAITING;
177+
},
178+
158179
displayIcon(): string {
159180
const exists = this.$store.getters['i18n/exists'];
160181
const t = this.$store.getters['i18n/t'];
@@ -204,10 +225,6 @@ export default defineComponent({
204225
return this.phase === ASYNC_BUTTON_STATES.WAITING;
205226
},
206227
207-
isDisabled(): boolean {
208-
return this.disabled || this.phase === ASYNC_BUTTON_STATES.WAITING;
209-
},
210-
211228
isManualRefresh() {
212229
return this.mode === 'manual-refresh';
213230
},
@@ -232,7 +249,7 @@ export default defineComponent({
232249
233250
methods: {
234251
clicked() {
235-
if ( this.isDisabled ) {
252+
if ( this.appearsDisabled ) {
236253
return;
237254
}
238255
@@ -283,8 +300,8 @@ export default defineComponent({
283300
:class="classes"
284301
:name="name"
285302
:type="type"
286-
:disabled="isDisabled"
287-
:aria-disabled="isDisabled"
303+
:disabled="disabled"
304+
:aria-disabled="appearsDisabled"
288305
:tab-index="tabIndex"
289306
:data-testid="componentTestid + '-async-button'"
290307
@click="clicked"

shell/components/__tests__/AsyncButton.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('component: AsyncButton', () => {
4242
expect(span.text()).toBe('some-string');
4343
});
4444

45-
it('click on async button should emit click with a proper state of waiting, disabled and spinning ::: CB true', () => {
45+
it('click on async button should emit click with a proper state of waiting, appear disabled and spinning ::: CB true', () => {
4646
jest.useFakeTimers();
4747

4848
const wrapper: VueWrapper<InstanceType<typeof AsyncButton>> = mount(AsyncButton, {
@@ -65,7 +65,7 @@ describe('component: AsyncButton', () => {
6565
expect(wrapper.emitted('click')).toHaveLength(1);
6666
expect(wrapper.vm.phase).toBe(ASYNC_BUTTON_STATES.WAITING);
6767
expect(wrapper.vm.isSpinning).toBe(true);
68-
expect(wrapper.vm.isDisabled).toBe(true);
68+
expect(wrapper.vm.appearsDisabled).toBe(true);
6969
// testing cb function has been emitted
7070
expect(typeof wrapper.emitted('click')![0][0]).toBe('function');
7171

0 commit comments

Comments
 (0)