Skip to content

Commit

Permalink
feat(dropdown): remove disabled attribute from clrDropdownItem
Browse files Browse the repository at this point in the history
Using the `disabled` attribute on a `clrDropdownItem` that is a `button`
element breaks keyboard navigation. Focus gets lost on the disabled item.

BREAKING CHANGE: Use `clrDisabled` instead of `disabled` to disable a `clrDropdownItem`.
  • Loading branch information
kevinbuhmann committed Dec 19, 2022
1 parent 3250027 commit 3346573
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 52 deletions.
7 changes: 1 addition & 6 deletions projects/angular/clarity.api.md
Expand Up @@ -2111,16 +2111,11 @@ export class ClrDropdownItem {
set disabled(value: boolean | string);
// (undocumented)
get disabled(): boolean | string;
set disabledDeprecated(value: boolean | string);
// (undocumented)
get disabledDeprecated(): boolean | string;
set dropdownItemId(value: string);
// (undocumented)
get dropdownItemId(): string;
// (undocumented)
setByDeprecatedDisabled: boolean;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<ClrDropdownItem, "[clrDropdownItem]", never, { "disabled": "clrDisabled"; "disabledDeprecated": "disabled"; "dropdownItemId": "id"; }, {}, never, never, false, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<ClrDropdownItem, "[clrDropdownItem]", never, { "disabled": "clrDisabled"; "dropdownItemId": "id"; }, {}, never, never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<ClrDropdownItem, never>;
}
Expand Down
29 changes: 0 additions & 29 deletions projects/angular/src/popover/dropdown/dropdown-item.spec.ts
Expand Up @@ -70,34 +70,5 @@ export default function (): void {
this.detectChanges();
expect(this.getClarityProvider(FocusableItem).disabled).toBe(false);
});

/*
* @deprecated since 3.0, remove in 4.0. tests for the deprecated [disabled] attribute
*/
it('sets the disabled attribute if set by input', function (this: Context) {
expect(this.clarityElement.getAttribute('disabled')).toBeNull();
this.hostComponent.disabledDeprecated = true;
this.detectChanges();
expect(this.clarityElement.getAttribute('disabled')).toBe('');
});

it('sets aria-disabled to true if the FocusableItem is disabled', function (this: Context) {
expect(this.clarityElement.getAttribute('aria-disabled')).toBe('false');
this.hostComponent.disabledDeprecated = true;
this.detectChanges();
expect(this.clarityElement.getAttribute('aria-disabled')).toBe('true');
});

it('updates the disabled property of the FocusableItem', function (this: Context) {
this.hostComponent.disabledDeprecated = true;
this.detectChanges();
expect(this.getClarityProvider(FocusableItem).disabled).toBe(true);
this.hostComponent.disabledDeprecated = false;
this.detectChanges();
expect(this.getClarityProvider(FocusableItem).disabled).toBe(false);
});
/*
* End @deprecated section
*/
});
}
17 changes: 0 additions & 17 deletions projects/angular/src/popover/dropdown/dropdown-item.ts
Expand Up @@ -18,7 +18,6 @@ import { RootDropdownService } from './providers/dropdown.service';
'[class.dropdown-item]': 'true',
'[attr.role]': '"menuitem"',
'[attr.aria-disabled]': 'disabled',
'[attr.disabled]': "(disabled && setByDeprecatedDisabled)? '' : null",
'[attr.id]': 'dropdownItemId',
},
providers: [BASIC_FOCUSABLE_ITEM_PROVIDER],
Expand All @@ -30,8 +29,6 @@ export class ClrDropdownItem {
private focusableItem: FocusableItem
) {}

setByDeprecatedDisabled = false;

@Input('clrDisabled')
set disabled(value: boolean | string) {
// Empty string attribute evaluates to false but should disable the item, so we need to add a special case for it.
Expand All @@ -42,20 +39,6 @@ export class ClrDropdownItem {
return this.focusableItem.disabled;
}

/*
* @deprecated since 3.0, remove in 4.0. the presence of this attribute makes it not-focusable in IE11. Use [clrDisabled] input instead.
*/
@Input('disabled')
set disabledDeprecated(value: boolean | string) {
// Empty string attribute evaluates to false but should disable the item, so we need to add a special case for it.
this.focusableItem.disabled = !!value || value === '';
this.setByDeprecatedDisabled = true;
}

get disabledDeprecated() {
return this.focusableItem.disabled;
}

/**
* Let you overwrite the focusable auto increment id.
*/
Expand Down

0 comments on commit 3346573

Please sign in to comment.