Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SFD-181: Implement ARIA tabs patterns #133

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion playwright/tests/test_17_EstTool_UI_AssumpLimitations.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_example(page: Page) -> None:
expect(page.get_by_text("Clearly there is a large")).to_be_visible()
expect(page.get_by_text("The estimated kWh of cloud")).to_be_visible()
expect(page.get_by_role("heading", name="Downstream Emissions")).to_be_visible()
expect(page.get_by_text("At present we focus on the")).to_be_visible()
expect(page.get_by_text("To do this we have collated")).to_be_visible()
Copy link
Contributor Author

@jantoun-scottlogic jantoun-scottlogic Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the panels are hidden when not selected rather than being removed from the DOM there are 2 elements in the DOM with the old text so I've used the start of the second sentence in the section instead.

expect(page.get_by_role("cell", name="Type", exact=True)).to_be_visible()
expect(page.get_by_text("These figures are combined")).to_be_visible()
expect(page.get_by_role("heading", name="Network Data Transfer")).to_be_visible()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<form (ngSubmit)="handleSubmit()" [formGroup]="estimatorForm" class="tce-w-full tce-flex tce-flex-col tce-gap-6">
<form
(ngSubmit)="handleSubmit()"
[formGroup]="estimatorForm"
class="tce-w-full tce-flex tce-flex-col tce-gap-6"
tabindex="0">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be in the tab sequence now that it's in a tabpanel. The other tabpanels are already behaving that way so I've added it here rather than on the actual tabpanel element since that was causing other panels to appear twice in the tab sequence.

<div formGroupName="upstream">
@if (errorSummaryState.showErrorSummary) {
<error-summary [validationErrors]="errorSummaryState.validationErrors"></error-summary>
Expand Down
4 changes: 2 additions & 2 deletions src/app/tab/tab-item/tab-item.component.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@if (active()) {
<div role="tabpanel" [hidden]="!active()" [id]="tabIdPrefix + 'Tabpanel'" [attr.aria-labelledby]="tabIdPrefix + 'Tab'">
<ng-content></ng-content>
}
</div>
1 change: 1 addition & 0 deletions src/app/tab/tab-item/tab-item.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('TabItemComponent', () => {
.compileComponents();

fixture = TestBed.createComponent(TabItemComponent);
fixture.componentRef.setInput('title', 'Tab Title');
component = fixture.componentInstance;
fixture.detectChanges();
});
Expand Down
10 changes: 8 additions & 2 deletions src/app/tab/tab-item/tab-item.component.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { CommonModule } from '@angular/common';
import { Component, effect, EventEmitter, input, model, Output } from '@angular/core';
import { Component, effect, EventEmitter, input, model, OnInit, Output } from '@angular/core';
import { camelCase } from 'lodash-es';

@Component({
selector: 'tab-item',
standalone: true,
imports: [CommonModule],
templateUrl: './tab-item.component.html',
})
export class TabItemComponent {
export class TabItemComponent implements OnInit {
public active = model(false);
public title = input.required<string>();
public tabIdPrefix!: string;
@Output() public tabSelected = new EventEmitter<void>();

constructor() {
Expand All @@ -19,4 +21,8 @@ export class TabItemComponent {
}
});
}

ngOnInit(): void {
this.tabIdPrefix = camelCase(this.title());
}
}
18 changes: 10 additions & 8 deletions src/app/tab/tabs/tabs.component.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<div class="tce-w-full tce-flex tce-mb-2" role="tablist">
@for (tab of tabs; track tab.title) {
<div
@for (tab of tabs; track tab.title()) {
<button
#tabButton
role="tab"
[id]="tab.tabIdPrefix + 'Tab'"
[attr.aria-controls]="tab.tabIdPrefix + 'Tabpanel'"
[class.tce-active-tab]="tab.active()"
[tabindex]="tab.active() ? 0 : -1"
[attr.aria-selected]="tab.active() ? true : false"
(click)="selectTab(tab)"
(keydown.enter)="selectTab(tab)"
tabindex="0"
(keydown)="onKeydown($event, $index)"
class="tce-p-2 tce-cursor-pointer tce-border-b-2 tce-border-transparent hover:tce-border-slate-300">
{{ tab.title() }}
</div>
</button>
}
</div>
<div role="tabpanel">
<ng-content></ng-content>
</div>
<ng-content></ng-content>
62 changes: 61 additions & 1 deletion src/app/tab/tabs/tabs.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AfterContentInit, Component, ContentChildren, QueryList } from '@angular/core';
import { AfterContentInit, Component, ContentChildren, ElementRef, QueryList, ViewChildren } from '@angular/core';
import { TabItemComponent } from '../tab-item/tab-item.component';

@Component({
Expand All @@ -9,6 +9,7 @@ import { TabItemComponent } from '../tab-item/tab-item.component';
})
export class TabsComponent implements AfterContentInit {
@ContentChildren(TabItemComponent) tabs!: QueryList<TabItemComponent>;
@ViewChildren('tabButton') tabButtons!: QueryList<ElementRef<HTMLButtonElement>>;

public ngAfterContentInit() {
const activeTabs = this.tabs.filter(tab => tab.active());
Expand All @@ -24,4 +25,63 @@ export class TabsComponent implements AfterContentInit {
this.tabs.filter(tab => tab.active()).forEach(tab => tab.active.set(false));
selectedTab.active.set(true);
}

selectAndFocusTab(selectedTabIndex: number) {
const newTab = this.tabs.get(selectedTabIndex)!;
const newTabButton = this.tabButtons.get(selectedTabIndex)!;
this.selectTab(newTab);
newTabButton.nativeElement.focus();
}

setSelectedToNextTab(currentTabIndex: number) {
let newTabIndex: number;
if (currentTabIndex === this.tabs.length - 1) {
newTabIndex = 0;
} else {
newTabIndex = currentTabIndex + 1;
}
this.selectAndFocusTab(newTabIndex);
}

setSelectedToPreviousTab(currentTabIndex: number) {
let newTabIndex: number;
if (currentTabIndex === 0) {
newTabIndex = this.tabs.length - 1;
} else {
newTabIndex = currentTabIndex - 1;
}
this.selectAndFocusTab(newTabIndex);
}

public onKeydown(event: KeyboardEvent, currentIndex: number) {
let flag = false;
switch (event.key) {
case 'ArrowRight':
flag = true;
this.setSelectedToNextTab(currentIndex);
break;

case 'ArrowLeft':
flag = true;
this.setSelectedToPreviousTab(currentIndex);
break;

case 'Home':
flag = true;
this.selectAndFocusTab(0);
break;

case 'End':
flag = true;
this.selectAndFocusTab(this.tabs.length - 1);
break;

default:
break;
}
if (flag) {
event.stopPropagation();
event.preventDefault();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ <h1 class="tce-text-3xl tce-mb-6">Technology Carbon Estimator</h1>
(formReset)="handleFormReset()"></carbon-estimator-form>
</tab-item>
<tab-item [title]="'Assumptions and Limitations'">
<assumptions-and-limitation aria-live="polite"></assumptions-and-limitation>
Copy link
Contributor Author

@jantoun-scottlogic jantoun-scottlogic Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally removed this when changing some other aria-live values but having reverted those changes and rechecked, I don't think this is changing the behaviour now although that could just be my screen reader settings. I've left it off for now since other elements placed directly inside the tab-item components do not have this set.

<assumptions-and-limitation></assumptions-and-limitation>
</tab-item>
</tabs>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ describe('TechCarbonEstimatorComponent', () => {
const formElement = fixture.nativeElement.querySelector('carbon-estimator-form');
const assumptionsElement = fixture.nativeElement.querySelector('assumptions-and-limitation');

expect(formElement).toBeTruthy();
expect(assumptionsElement).toBeFalsy();
expect(formElement.checkVisibility()).toBeTrue();
expect(assumptionsElement.checkVisibility()).toBeFalse();
});

it('should call estimationService.calculateCarbonEstimation when handleFormSubmit is called', () => {
Expand Down