Skip to content

Commit

Permalink
fix(navigation): sync state with newly added nav items
Browse files Browse the repository at this point in the history
fixes  #155 #82

- previously only nav items in the initial render were synced with the top level expanded state
- add onslotchange to check for that
- move tabindex logic to slot change handler as well
- update unit tests to check syncing after component updates
  • Loading branch information
Ashley Ryan authored and ashleyryan committed Aug 24, 2022
1 parent 292df50 commit 6c48eff
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 9 deletions.
6 changes: 5 additions & 1 deletion projects/core/custom-elements.json
Expand Up @@ -50012,7 +50012,11 @@
},
{
"kind": "method",
"name": "addStartEventListener"
"name": "onStartItemSlotChange"
},
{
"kind": "method",
"name": "onItemSlotChange"
},
{
"kind": "method",
Expand Down
38 changes: 38 additions & 0 deletions projects/core/src/navigation/navigation.element.spec.ts
Expand Up @@ -125,6 +125,12 @@ describe('cds-navigation', () => {
itemRefs.forEach(item => {
expect(item.expanded).toBe(component.expanded);
});

component.expanded = true;
await componentIsStable(component);
itemRefs.forEach(item => {
expect(item.expanded).toBe(component.expanded);
});
});

it('to navigationStartRefs', async () => {
Expand All @@ -133,12 +139,22 @@ describe('cds-navigation', () => {
startRefs.forEach(start => {
expect(start.expandedRoot).toBe(component.expandedRoot);
});

component.expanded = true;
await componentIsStable(component);
startRefs.forEach(start => {
expect(start.expandedRoot).toBe(component.expandedRoot);
});
});

it('to rootNavigationStart', async () => {
await componentIsStable(component);
const rootStart = component.querySelector<CdsNavigationStart>(':scope > cds-navigation-start');
expect(rootStart.expanded).toBe(component.expanded);

component.expanded = true;
await componentIsStable(component);
expect(rootStart.expanded).toBe(component.expanded);
});

it('to rootNavigationItems', async () => {
Expand All @@ -147,6 +163,28 @@ describe('cds-navigation', () => {
rootItems.forEach(item => {
expect(item.expanded).toBe(component.expanded);
});

component.expanded = true;
await componentIsStable(component);
rootItems.forEach(item => {
expect(item.expanded).toBe(component.expanded);
});
});

it('to new navigation items', async () => {
component.expanded = true;
await componentIsStable(component);

const navItem = document.createElement('cds-navigation-item');
navItem.id = 'my-new-nav-item';
navItem.innerHTML = '<a href="#">My New Nav Item</a>';

component.appendChild(navItem);

await componentIsStable(component);

expect(navItem.expanded).toBe(component.expanded);
expect(navItem.tabIndex).toBe(-1);
});
});
});
23 changes: 15 additions & 8 deletions projects/core/src/navigation/navigation.element.ts
Expand Up @@ -181,7 +181,7 @@ export class CdsNavigation extends LitElement {
return this.navigationEnd
? html`
<div class="navigation-end" cds-layout="vertical align:horizontal-stretch">
<slot name="cds-navigation-end"></slot>
<slot name="cds-navigation-end" @slotchange=${this.onItemSlotChange}></slot>
</div>
`
: '';
Expand All @@ -193,7 +193,7 @@ export class CdsNavigation extends LitElement {
this.rootNavigationStart
? (returnHTML = html`
<div class="navigation-start" cds-layout="vertical align:horizontal-stretch">
<slot @slotchange="${() => this.addStartEventListener()}" name="navigation-start"></slot>
<slot @slotchange="${() => this.onStartItemSlotChange()}" name="navigation-start"></slot>
<cds-divider class="start-divider"></cds-divider>
</div>
`)
Expand All @@ -209,7 +209,7 @@ export class CdsNavigation extends LitElement {
<nav class="navigation-body-wrapper">
<div .ariaActiveDescendant=${this.ariaActiveDescendant} tabindex="0" id="item-container">
<div class="navigation-body" cds-layout="vertical wrap:none align:horizontal-stretch">
<slot></slot>
<slot @slotchange=${this.onItemSlotChange}></slot>
</div>
</div>
</nav>
Expand All @@ -234,10 +234,6 @@ export class CdsNavigation extends LitElement {

firstUpdated(props: PropertyValues<this>) {
super.firstUpdated(props);
// set all visible navigation elements to tabindex = -1
this.allNavigationElements.forEach(item => {
setAttributes(item, ['tabindex', '-1']);
});

const itemWrapper = this.shadowRoot?.querySelector('#item-container');
itemWrapper?.addEventListener('focus', this.initItemKeys.bind(this));
Expand All @@ -255,14 +251,25 @@ export class CdsNavigation extends LitElement {
this.updateChildrenProps();
}

addStartEventListener() {
onStartItemSlotChange() {
this.onItemSlotChange();

// This is controlled by the slotchange event on the navigation-start slot
// Make sure we reattach the click handler for toggle if consumer changes the start element (e.g *ngIf)
if (this.rootNavigationStart) {
this.rootNavigationStart.addEventListener('click', this.toggle.bind(this));
}
}

onItemSlotChange() {
this.updateChildrenProps();

// set all visible navigation elements to tabindex = -1
this.allNavigationElements.forEach(item => {
setAttributes(item, ['tabindex', '-1']);
});
}

updateChildrenProps() {
if (this.navigationGroupItems) {
syncPropsForAllItems(Array.from(this.navigationGroupItems), this, { groupItem: true });
Expand Down

0 comments on commit 6c48eff

Please sign in to comment.