Skip to content
Merged
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
11 changes: 9 additions & 2 deletions src/aria/accordion/accordion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,19 @@ describe('AccordionGroup', () => {
});

it('should not allow keyboard navigation if group is disabled', () => {
configureAccordionComponent({disabledGroup: true});
configureAccordionComponent({disabledGroup: true, softDisabled: false});

downArrowKey(triggerElements[0]);
expect(isTriggerActive(triggerElements[1])).toBeFalse();
});

it('should allow keyboard navigation if group is disabled', () => {
configureAccordionComponent({disabledGroup: true});

downArrowKey(triggerElements[0]);
expect(isTriggerActive(triggerElements[1])).toBeTrue();
});

it('should not allow expansion if group is disabled', () => {
configureAccordionComponent({disabledGroup: true});

Expand Down Expand Up @@ -419,7 +426,7 @@ class AccordionGroupExample {
value = model<string[]>([]);
multiExpandable = signal(false);
disabledGroup = signal(false);
softDisabled = signal(false);
softDisabled = signal(true);
wrap = signal(false);

disableItem(itemValue: string, disabled: boolean) {
Expand Down
2 changes: 1 addition & 1 deletion src/aria/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export class AccordionGroup {
value = model<string[]>([]);

/** Whether to allow disabled items to receive focus. */
softDisabled = input(false, {transform: booleanAttribute});
softDisabled = input(true, {transform: booleanAttribute});

/** Whether keyboard navigation should wrap around from the last item to the first, and vice-versa. */
wrap = input(false, {transform: booleanAttribute});
Expand Down
2 changes: 1 addition & 1 deletion src/aria/grid/grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class Grid {
readonly disabled = input(false, {transform: booleanAttribute});

/** Whether to allow disabled items to receive focus. */
readonly softDisabled = input(false, {transform: booleanAttribute});
readonly softDisabled = input(true, {transform: booleanAttribute});

/** The focus strategy used by the grid. */
readonly focusMode = input<'roving' | 'activedescendant'>('roving');
Expand Down
62 changes: 52 additions & 10 deletions src/aria/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,16 @@ describe('Listbox', () => {
expect(listboxElement.getAttribute('tabindex')).toBe('-1');
});

it('should set tabindex="0" for the listbox when disabled and focusMode is "roving"', () => {
setupListbox({disabled: true, focusMode: 'roving'});
it('should set tabindex="0" for the listbox when disabled and focusMode is "roving when softDisabled is false"', () => {
setupListbox({disabled: true, focusMode: 'roving', softDisabled: false});
expect(listboxElement.getAttribute('tabindex')).toBe('0');
});

it('should set tabindex="-1" for the listbox when disabled and focusMode is "roving"', () => {
setupListbox({disabled: true, focusMode: 'roving'});
expect(listboxElement.getAttribute('tabindex')).toBe('-1');
});

it('should set initial focus (tabindex="0") on the first non-disabled option if no value is set', () => {
setupListbox({focusMode: 'roving'});
expect(optionElements[0].getAttribute('tabindex')).toBe('0');
Expand All @@ -218,8 +223,23 @@ describe('Listbox', () => {
expect(optionElements[4].getAttribute('tabindex')).toBe('-1');
});

it('should set initial focus (tabindex="0") on the first non-disabled option if selected option is disabled', () => {
setupListbox({focusMode: 'roving', value: [1], disabledOptions: [1]});
it('should set initial focus (tabindex="0") on the first non-disabled option if selected option is disabled when softDisabled is false', () => {
setupListbox({
focusMode: 'roving',
value: [1],
disabledOptions: [0],
softDisabled: false,
});
expect(optionElements[0].getAttribute('tabindex')).toBe('-1');
expect(optionElements[1].getAttribute('tabindex')).toBe('0');
});

it('should set initial focus (tabindex="0") on the first option if selected option is disabled', () => {
setupListbox({
focusMode: 'roving',
value: [0],
disabledOptions: [0],
});
expect(optionElements[0].getAttribute('tabindex')).toBe('0');
expect(optionElements[1].getAttribute('tabindex')).toBe('-1');
});
Expand Down Expand Up @@ -247,10 +267,20 @@ describe('Listbox', () => {
});

it('should set aria-activedescendant to the ID of the first non-disabled option if selected option is disabled', () => {
setupListbox({focusMode: 'activedescendant', value: [1], disabledOptions: [1]});
setupListbox({focusMode: 'activedescendant', value: [0], disabledOptions: [0]});
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[0].id);
});

it('should set aria-activedescendant to the ID of the first non-disabled option if selected option is disabled when softDisabled is false', () => {
setupListbox({
focusMode: 'activedescendant',
value: [1],
disabledOptions: [0],
softDisabled: false,
});
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[1].id);
});

it('should set tabindex="-1" for all options', () => {
setupListbox({focusMode: 'activedescendant'});
expect(optionElements[0].getAttribute('tabindex')).toBe('-1');
Expand Down Expand Up @@ -553,17 +583,17 @@ describe('Listbox', () => {
isFocused: (index: number) => boolean,
) {
describe(`keyboard navigation (focusMode="${focusMode}")`, () => {
it('should move focus to the last enabled option on End', () => {
it('should move focus to the last focusable option on End', () => {
setupListbox({focusMode, disabledOptions: [4]});
end();
expect(isFocused(3)).toBe(true);
expect(isFocused(4)).toBe(true);
});

it('should move focus to the first enabled option on Home', () => {
it('should move focus to the first focusable option on Home', () => {
setupListbox({focusMode, disabledOptions: [0]});
end();
home();
expect(isFocused(1)).toBe(true);
expect(isFocused(0)).toBe(true);
});

it('should allow keyboard navigation if the group is readonly', () => {
Expand Down Expand Up @@ -614,6 +644,18 @@ describe('Listbox', () => {
down();
expect(isFocused(1)).toBe(true);
});

it('should not skip disabled options with ArrowDown when completely disabled', () => {
setupListbox({
focusMode,
orientation: 'vertical',
softDisabled: true,
disabled: true,
});

down();
expect(isFocused(0)).toBe(true);
});
});

describe('horizontal orientation', () => {
Expand Down Expand Up @@ -774,7 +816,7 @@ class ListboxExample {
value: number[] = [];
disabled = false;
readonly = false;
softDisabled = false;
softDisabled = true;
focusMode: 'roving' | 'activedescendant' = 'roving';
orientation: 'vertical' | 'horizontal' = 'vertical';
multi = false;
Expand Down
2 changes: 1 addition & 1 deletion src/aria/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class Listbox<V> {
wrap = input(true, {transform: booleanAttribute});

/** Whether to allow disabled items in the list to receive focus. */
softDisabled = input(false, {transform: booleanAttribute});
softDisabled = input(true, {transform: booleanAttribute});

/** The focus strategy used by the list. */
focusMode = input<'roving' | 'activedescendant'>('roving');
Expand Down
2 changes: 1 addition & 1 deletion src/aria/private/accordion/accordion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Accordion Pattern', () => {
multiExpandable: signal(true),
items: signal([]),
expandedIds: signal<string[]>([]),
softDisabled: signal(false),
softDisabled: signal(true),
wrap: signal(true),
element: signal(document.createElement('div')),
};
Expand Down
2 changes: 1 addition & 1 deletion src/aria/private/behaviors/grid/grid-focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class GridFocus<T extends GridFocusCell> {

/** Moves focus to the cell at the given coordinates if it's part of a focusable cell. */
focusCoordinates(coords: RowCol): boolean {
if (this.gridDisabled()) {
if (this.gridDisabled() && !this.inputs.softDisabled()) {
Copy link
Member

Choose a reason for hiding this comment

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

I took a look at list-focus and it does not allow focusing on items when a list is disabled https://github.com/angular/components/blob/main/src/aria/private/behaviors/list-focus/list-focus.ts#L101 and can test with https://ng-comp-devapp.web.app/aria-listbox by setting both disabled and softDisabled to true.

so I think we should align the implementation between those two, either

  1. If a grid/list is disabled, then no cells/items can be focused when softDisabled sets to true, or
  2. if a grid/list is disabled, cells/items can still be focused when softDisabled sets to true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After chatting we discussed that it's probably best to keep them all focusable when softDisabled is set to true. So for consistency I have updated listbox.

return false;
}

Expand Down
Loading
Loading