From a8dcf432f1327601d8ff596df4db5baa28770239 Mon Sep 17 00:00:00 2001 From: Ulrich Stellmacher Date: Thu, 17 Oct 2024 15:24:43 +0200 Subject: [PATCH 1/2] fix: CXSPA-7931 - Group Tree is not part of tab chain In cases where the currently displayed group is neither a tree node in the navigation menu itself, nor a direct child of one of the navigation tree nodes, but an indirect child of one of the navigation tree nodes, the tree can't be reached by tabbing anymore. The associated check is enhance to work recursively, to also detect the above described situation properly. --- .../configurator-group-menu.component.spec.ts | 39 +++++++++++++++++++ .../configurator-group-menu.component.ts | 29 ++++++-------- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.spec.ts b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.spec.ts index 6d01bed8e7d..3beec7aa094 100644 --- a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.spec.ts +++ b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.spec.ts @@ -36,6 +36,7 @@ import { GROUP_ID_4, GROUP_ID_5, GROUP_ID_7, + GROUP_ID_8, PRODUCT_CODE, mockRouterState, productConfiguration, @@ -1204,6 +1205,44 @@ describe('ConfiguratorGroupMenuComponent', () => { ) ).toBe(true); }); + + it('should return `true` because a group with current group ID is part of the sub group hierarchy', () => { + expect( + component.containsSelectedGroup( + mockProductConfiguration.groups[3], // GROUP_ID_5 is parent of GROUP_ID_7 which in turn is parent of GROUP_ID_8 + GROUP_ID_8 + ) + ).toBe(true); + }); + }); + + describe('getTabIndex', () => { + it('should return `0` because current group id matches group itself', () => { + expect( + component.getTabIndex(mockProductConfiguration.groups[0], GROUP_ID_1) + ).toBe(0); + }); + + it("should return `-1` because current group id doesn't match group or its children", () => { + expect( + component.getTabIndex(mockProductConfiguration.groups[0], GROUP_ID_4) + ).toBe(-1); + }); + + it('should return `0` because current group id matches a direct child group id', () => { + expect( + component.getTabIndex(mockProductConfiguration.groups[2], GROUP_ID_4) + ).toBe(0); + }); + + it('should return `0` because current group id matches a in-direct child group id', () => { + expect( + component.getTabIndex( + mockProductConfiguration.groups[3], // GROUP_ID_5 is parent of GROUP_ID_7 which in turn is parent of GROUP_ID_8 + GROUP_ID_8 + ) + ).toBe(0); + }); }); describe('isGroupSelected', () => { diff --git a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts index 1656a0993ed..f077112b692 100644 --- a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts +++ b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts @@ -18,12 +18,12 @@ import { ConfiguratorRouterExtractorService, } from '@spartacus/product-configurator/common'; import { + BREAKPOINT, + BreakpointService, DirectionMode, DirectionService, HamburgerMenuService, ICON_TYPE, - BREAKPOINT, - BreakpointService, } from '@spartacus/storefront'; import { Observable, of } from 'rxjs'; import { filter, map, switchMap, take } from 'rxjs/operators'; @@ -503,13 +503,12 @@ export class ConfiguratorGroupMenuComponent { group: Configurator.Group, currentGroupId?: string ): boolean { - let isCurrentGroupFound = false; - group.subGroups?.forEach((subGroup) => { - if (this.isGroupSelected(subGroup.id, currentGroupId)) { - isCurrentGroupFound = true; - } - }); - return isCurrentGroupFound; + let groupIdx = group.subGroups?.findIndex( + (subGroup) => + this.isGroupSelected(subGroup.id, currentGroupId) || + this.containsSelectedGroup(subGroup, currentGroupId) + ); + return groupIdx !== -1; } /** @@ -521,14 +520,10 @@ export class ConfiguratorGroupMenuComponent { * @returns {number} - tab index */ getTabIndex(group: Configurator.Group, currentGroupId: string): number { - if ( - !this.isGroupSelected(group.id, currentGroupId) && - !this.containsSelectedGroup(group, currentGroupId) - ) { - return -1; - } else { - return 0; - } + const isCurrentGroupPartOfGroupHierarchy = + this.isGroupSelected(group.id, currentGroupId) || + this.containsSelectedGroup(group, currentGroupId); + return isCurrentGroupPartOfGroupHierarchy ? 0 : -1; // 0 -> add to tab chain, -1 -> remove from tab chain } /** From 698af6e4c55227a85e5e677cd8a829a11acc099b Mon Sep 17 00:00:00 2001 From: Ulrich Stellmacher Date: Thu, 17 Oct 2024 15:42:43 +0200 Subject: [PATCH 2/2] refactor: simplify expression --- .../components/group-menu/configurator-group-menu.component.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts index f077112b692..16f2641f74e 100644 --- a/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts +++ b/feature-libs/product-configurator/rulebased/components/group-menu/configurator-group-menu.component.ts @@ -503,12 +503,11 @@ export class ConfiguratorGroupMenuComponent { group: Configurator.Group, currentGroupId?: string ): boolean { - let groupIdx = group.subGroups?.findIndex( + return !!group.subGroups?.find( (subGroup) => this.isGroupSelected(subGroup.id, currentGroupId) || this.containsSelectedGroup(subGroup, currentGroupId) ); - return groupIdx !== -1; } /**