Skip to content

Commit

Permalink
fix(dropdown): set tabIndex to -1 on disabled dropdown items
Browse files Browse the repository at this point in the history
This makes sure that disabled dropdown items using links are not tab-traversable.

fix #4301
  • Loading branch information
jnizet authored and maxokorokov committed Apr 6, 2022
1 parent 4f71568 commit 1ab5d21
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
22 changes: 19 additions & 3 deletions src/dropdown/dropdown.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,22 +300,37 @@ describe('ngb-dropdown', () => {
});

it('should disable a button dropdown item', () => {
const html = `<button ngbDropdownItem [disabled]="true">dropDown item</button>`;
const html = `<button ngbDropdownItem [disabled]="disabled">dropDown item</button>`;

const fixture = createTestComponent(html);
const itemEl = fixture.nativeElement.querySelector('button');

expect(itemEl).not.toHaveCssClass('disabled');
expect(itemEl.disabled).toBeFalse();
expect(itemEl.tabIndex).toBe(0);

fixture.componentInstance.disabled = true;
fixture.detectChanges();

expect(itemEl).toHaveCssClass('disabled');
expect(itemEl.disabled).toBe(true);
expect(itemEl.disabled).toBeTrue();
expect(itemEl.tabIndex).toBe(-1);
});

it('should disable a link dropdown item', () => {
const html = `<a ngbDropdownItem [disabled]="true">dropDown item</a>`;
const html = `<a ngbDropdownItem [disabled]="disabled">dropDown item</a>`;

const fixture = createTestComponent(html);
const itemEl = fixture.nativeElement.querySelector('a');

expect(itemEl).not.toHaveCssClass('disabled');
expect(itemEl.tabIndex).toBe(0);

fixture.componentInstance.disabled = true;
fixture.detectChanges();

expect(itemEl).toHaveCssClass('disabled');
expect(itemEl.tabIndex).toBe(-1);
});
});

Expand Down Expand Up @@ -543,6 +558,7 @@ class TestComponent {
isOpen = false;
stateChanges: boolean[] = [];
dropdownClass = 'custom-class';
disabled = false;

recordStateChange($event) {
this.stateChanges.push($event);
Expand Down
8 changes: 7 additions & 1 deletion src/dropdown/dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ export class NgbNavbar {
*
* @since 4.1.0
*/
@Directive({selector: '[ngbDropdownItem]', host: {'class': 'dropdown-item', '[class.disabled]': 'disabled'}})
@Directive({
selector: '[ngbDropdownItem]',
host: {'class': 'dropdown-item', '[class.disabled]': 'disabled', '[tabIndex]': 'disabled ? -1 : 0'}
})
export class NgbDropdownItem {
static ngAcceptInputType_disabled: boolean | '';

Expand All @@ -49,6 +52,9 @@ export class NgbDropdownItem {
@Input()
set disabled(value: boolean) {
this._disabled = <any>value === '' || value === true; // accept an empty attribute as true
// note: we don't use a host binding for disabled because when used on links, it fails because links don't have a
// disabled property
// setting the property using the renderer, OTOH, works fine in both cases.
this._renderer.setProperty(this.elementRef.nativeElement, 'disabled', this._disabled);
}

Expand Down

0 comments on commit 1ab5d21

Please sign in to comment.