Skip to content

Commit

Permalink
Add some accessibility attributes to context menus
Browse files Browse the repository at this point in the history
- Add optional label to context menu.
- Add menuitem and separator role attribute to context menu items.
- Use capybara accessible selectors to interact with them in tests.
  • Loading branch information
cbliard committed Dec 26, 2023
1 parent 7e9b5cb commit bd74da2
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 19 deletions.
1 change: 1 addition & 0 deletions config/locales/js-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ en:
label_watcher_added_successfully: "Watcher successfully added!"
label_watcher_deleted_successfully: "Watcher successfully deleted!"
label_work_package_details_you_are_here: "You are on the %{tab} tab for %{type} %{subject}."
label_work_package_context_menu: "Work package context menu"
label_unwatch: "Unwatch"
label_unwatch_work_package: "Unwatch work package"
label_uploaded_by: "Uploaded by"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
ng-class="{'dropdown-anchor-right': locals.showAnchorRight}">
<ul class="dropdown-menu "
role="menu"
[attr.aria-label]="locals.label"
[ngClass]="{'-empty': items.length === 0 }">
<ng-container *ngFor="let item of items">
<li *ngIf="item.divider" class="dropdown-divider"></li>
<li *ngIf="item.divider" class="dropdown-divider" role="separator"></li>
<li *ngIf="!item.divider">
<a
*ngIf="item.href"
class="menu-item"
role="menuitem"
[ngClass]="item.class"
[class.inactive]="item.disabled"
[attr.href]="item.href"
Expand All @@ -27,6 +29,7 @@
<button
*ngIf="!item.href"
class="menu-item"
role="menuitem"
[ngClass]="item.class"
[class.inactive]="item.disabled"
[attr.aria-disabled]="item.disabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import { InjectionToken } from '@angular/core';

export const OpContextMenuLocalsToken = new InjectionToken<any>('CONTEXT_MENU_LOCALS');

export interface OpContextMenuLocalsMap {
items:OpContextMenuItem[];
contextMenuId?:string;
[key:string]:any;
}

export interface OpContextMenuItem {
disabled?:boolean;
hidden?:boolean;
Expand All @@ -20,3 +14,11 @@ export interface OpContextMenuItem {
divider?:boolean;
onClick?:($event:JQuery.TriggeredEvent|MouseEvent) => boolean;
}

export interface OpContextMenuLocalsMap {
items:OpContextMenuItem[];
showAnchorRight?:boolean;
contextMenuId?:string;
label?:string;
[key:string]:any;

Check warning on line 23 in frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts

View workflow job for this annotation

GitHub Actions / eslint

[eslint] frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts#L23 <@typescript-eslint/no-explicit-any>(https://typescript-eslint.io/rules/no-explicit-any)

Unexpected any. Specify a different type.
Raw output
{"ruleId":"@typescript-eslint/no-explicit-any","severity":1,"message":"Unexpected any. Specify a different type.","line":23,"column":16,"nodeType":"TSAnyKeyword","messageId":"unexpectedAny","endLine":23,"endColumn":19,"suggestions":[{"messageId":"suggestUnknown","fix":{"range":[566,569],"text":"unknown"},"desc":"Use `unknown` instead, this will force you to explicitly, and safely assert the type is correct."},{"messageId":"suggestNever","fix":{"range":[566,569],"text":"never"},"desc":"Use `never` instead, this is useful when instantiating generic type parameters that you don't need to know the type of."}]}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,23 @@ export class WorkPackageViewContextMenu extends OpContextMenuHandler {

private copyToClipboardService:CopyToClipboardService;

constructor(public injector:Injector,
constructor(
public injector:Injector,
protected workPackageId:string,
protected $element:JQuery,
protected additionalPositionArgs:any = {},
protected allowSplitScreenActions:boolean = true) {
protected allowSplitScreenActions:boolean = true,
) {
super(injector.get(OPContextMenuService));
this.copyToClipboardService = injector.get(CopyToClipboardService);
}

public get locals():OpContextMenuLocalsMap {
return { contextMenuId: 'work-package-context-menu', items: this.items };
return {
contextMenuId: 'work-package-context-menu',
label: I18n.t('js.label_work_package_context_menu'),
items: this.items,
};
}

public positionArgs(evt:JQuery.TriggeredEvent) {
Expand Down
28 changes: 19 additions & 9 deletions spec/support/components/work_packages/context_menu.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,17 @@ def open_for(work_package, card_view: nil)
end

def expect_open
expect(page).to have_selector(selector)
expect(page).to have_selector(:menu, work_package_context_menu_label)
end

def expect_closed
expect(page).not_to have_selector(selector)
expect(page).not_to have_selector(:menu, work_package_context_menu_label)
end

def choose(target)
find("#{selector} .menu-item", text: target, exact_text: true).click
within_menu do
find(:menuitem, text: target, exact_text: true).click
end
end

def choose_delete_and_confirm_deletion
Expand All @@ -83,22 +85,30 @@ def choose_delete_and_confirm_deletion

def expect_no_options(*options)
expect_open
options.each do |text|
expect(page).not_to have_selector("#{selector} .menu-item", text:)
within_menu do
options.each do |text|
expect(page).not_to have_selector(:menuitem, text:)
end
end
end

def expect_options(options)
expect_open
options.each do |text|
expect(page).to have_selector("#{selector} .menu-item", text:)
within_menu do
options.each do |text|
expect(page).to have_selector(:menuitem, text:)
end
end
end

private

def selector
'#work-package-context-menu'
def within_menu(&)
within(:menu, work_package_context_menu_label, &)
end

def work_package_context_menu_label
I18n.t('js.label_work_package_context_menu')
end
end
end
Expand Down

0 comments on commit bd74da2

Please sign in to comment.