Skip to content

Commit

Permalink
fix: Check availability of config form before setting focus (#12342)
Browse files Browse the repository at this point in the history
When the focus was set on previous/next navigation, the interactive configuration form was sometimes already teared down. This led to an error message in the console

Closes #12341
  • Loading branch information
ChristophHi authored May 10, 2021
1 parent fa49838 commit 7d54810
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('ConfigUtilsService', () => {
'testProduct'
);
let keyboardFocusService: KeyboardFocusService;
let querySelectorOriginal: any;

beforeEach(() => {
TestBed.configureTestingModule({
Expand All @@ -48,6 +49,11 @@ describe('ConfigUtilsService', () => {
keyboardFocusService = TestBed.inject(
KeyboardFocusService as Type<KeyboardFocusService>
);
querySelectorOriginal = document.querySelector;
});

afterEach(() => {
document.querySelector = querySelectorOriginal;
});

it('should be created', () => {
Expand Down Expand Up @@ -117,24 +123,30 @@ describe('ConfigUtilsService', () => {
});

describe('focusFirstAttribute', () => {
it('should return no focused attribute because keyboardFocusService is undefined', () => {
it('should not delegate to keyboardFocusService if we did not provide that', () => {
classUnderTest['keyboardFocusService'] = undefined;
spyOn(keyboardFocusService, 'findFocusable').and.stub();
classUnderTest.focusFirstAttribute();
expect(keyboardFocusService.findFocusable).toHaveBeenCalledTimes(0);
});

it('should return no focused attribute because keyboardFocusService is null', () => {
classUnderTest['keyboardFocusService'] = null;
spyOn(keyboardFocusService, 'findFocusable').and.stub();
it('should delegate to focus service', () => {
const theElement = document.createElement('form');
document.querySelector = jasmine
.createSpy('HTML Element')
.and.returnValue(theElement);
spyOn(keyboardFocusService, 'findFocusable').and.returnValue([]);
classUnderTest.focusFirstAttribute();
expect(keyboardFocusService.findFocusable).toHaveBeenCalledTimes(0);
expect(keyboardFocusService.findFocusable).toHaveBeenCalledTimes(1);
});

it('should return no focused attribute because there is no found', () => {
it('should not delegate to focus service if form is not available', () => {
document.querySelector = jasmine
.createSpy('HTML Element')
.and.returnValue(null);
spyOn(keyboardFocusService, 'findFocusable').and.returnValue([]);
classUnderTest.focusFirstAttribute();
expect(keyboardFocusService.findFocusable).toHaveBeenCalledTimes(1);
expect(keyboardFocusService.findFocusable).toHaveBeenCalledTimes(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,16 @@ export class ConfiguratorStorefrontUtilsService {
focusFirstAttribute(): void {
if (this.keyboardFocusService) {
if (isPlatformBrowser(this.platformId)) {
const form: HTMLElement = document.querySelector(
const form: HTMLElement | null = document.querySelector(
'cx-configurator-form'
);
const focusedElements: HTMLElement[] = this.keyboardFocusService.findFocusable(
form
);
if (focusedElements && focusedElements.length > 1) {
focusedElements[0]?.focus();
if (form) {
const focusableElements: HTMLElement[] = this.keyboardFocusService.findFocusable(
form
);
if (focusableElements && focusableElements.length > 0) {
focusableElements[0].focus();
}
}
}
}
Expand Down

0 comments on commit 7d54810

Please sign in to comment.