Skip to content

Commit

Permalink
fix: CXSPA-7931 - Group Tree is not part of tab chain
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Uli-Tiger committed Oct 17, 2024
1 parent 35a1751 commit a8dcf43
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
GROUP_ID_4,
GROUP_ID_5,
GROUP_ID_7,
GROUP_ID_8,
PRODUCT_CODE,
mockRouterState,
productConfiguration,
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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
}

/**
Expand Down

0 comments on commit a8dcf43

Please sign in to comment.