NileshPatel17 / ng-multiselect-dropdown

Multiple Select Dropdown Component
https://nileshpatel17.github.io/ng-multiselect-dropdown/
327 stars 288 forks source link

Disabled property still allows to open the dropdown. #179

Open Trixt0r opened 5 years ago

Trixt0r commented 5 years ago

When setting the property disabled to true, the dropdown still reveals if you click on the disabled element.

Also the mouse cursor is not correct in this situation. It should be set to no-drop.

Code example:

<ng-multiselect-dropdown
  [settings]="{idField: 'id', textField: 'id' }"
  [data]="[{id: 'one'}, {id: 'two'}, {id: 'three'}]"
  [disabled]="true">
</ng-multiselect-dropdown>

See a full exmaple here: https://codesandbox.io/s/ngmultiselectdropdownmodule-4g36p

NileshPatel17 commented 5 years ago

I ran through your code sample, and it works as expected. disabled property does not allow to select/check all/individual item. but still it allows to open the dropdown. let me know your thought.

Trixt0r commented 5 years ago

Really? This is what I get, if I run the codesandbox example:

multiselect-disabled

It can't work, since you are not checking for the disabled flag on its own:

https://github.com/NileshPatel17/ng-multiselect-dropdown/blob/master/src/ng-multiselect-dropdown/src/multiselect.component.ts#L276

You have to implement it like this:

if (this.disabled) return;

I had to extend the component anyway, since the "select all" and "deselect all" options do not behave as expected for disabled items. Disabled items still get selected and unselected.

NileshPatel17 commented 5 years ago

method toggleDropdown() at line no 276 is not being used. It should be removed. Anyway,I will do that.

There is a check if( this.disabled) already. line no: 115(on Item selection)

onItemClick($event: any, item: ListItem) {
    if (this.disabled || item.isDisabled) {
      return false;
    }

line no: 296(on "select all/unselect all"

 toggleSelectAll() {
    if (this.disabled) {
      return false;
    }

Let me know still you think, it dos not work. Please share code sample with steps to reproduce. I will cross-check if i have test case for this is in place. if it does not, will add.

NileshPatel17 commented 5 years ago

my bad.. i may have missed it. disable property allow to open the dropdown, but does not allow to select an item or all items. that's how it is intended to work.

Vaibhav2812 commented 5 years ago

line no 284: -

` toggleDropdown(evt) {
evt.preventDefault();
if (this.disabled && this._settings.singleSelection) {
  return;
}
this._settings.defaultOpen = !this._settings.defaultOpen;
if (!this._settings.defaultOpen) {
  this.onDropDownClose.emit();
}

}`

replace if (this.disabled && this._settings.singleSelection) { return; } with if (this.disabled) { return; }

it will work.

NileshPatel17 commented 5 years ago

toggleDropdown() never being used. I will be removing it.

Trixt0r commented 5 years ago

It is used.

my bad.. i may have missed it. disable property allow to open the dropdown, but does not allow to select an item or all items. that's how it is intended to work.

You can't be serious. Disabled means disabled. A disabled component is not allowed to receive any user input and react to any of it. Never saw a disabled dropdown which shows its items and does not allow to select them.

sarunci commented 3 years ago

I also ran into the same issue. In case of single select, when disabled, the search filters and dropdown are not shown. In case of multiselect, when disabled, the search filter and dropdown list are displayed. The only scenario I can think of is, if there are multiple values already selected, then the dropdown will help to display the values selected.

Maybe we can modify the behavior for multiselect. If disabled and no values are selected, do not display the search filter and dropdown. If disabled and values are already selected, then the dropdown can show the selected values only.

giuliovivian-interlogica commented 2 years ago

Workaround: (after entering the disabled property to the ng-multiselect-dropdown tag)

.multiselect-dropdown { .disabled .dropdown-btn { background-color: $grey_light; cursor: not-allowed; } .disabled + .dropdown-list { display: none; } }

nicolaszaatar commented 2 years ago

Any updates on this feature?

nicolaszaatar commented 2 years ago

For anyone who's still lost here, check this tutorial and insert whatever CSS changes in the copied file.

https://github.com/NileshPatel17/ng-multiselect-dropdown/blob/master/custom-theme.md

JHarrisGTI commented 1 year ago

To comply with accessibility standards, disabling the control should also remove it from being tab-selected. This could be achieved by setting tabindex: -1 on the relevant <input>.