Skip to content

fix: support disabling options in multi-select #979

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 9, 2021
Merged

fix: support disabling options in multi-select #979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff00d8d
fix: multi select support disabled options wip
arjunlalb Jul 8, 2021
a3855da
fix: add test
arjunlalb Jul 8, 2021
98ced07
fix: prettier fix
arjunlalb Jul 8, 2021
25043ed
fix: fix lint issue
arjunlalb Jul 8, 2021
72ef3bb
Merge branch 'main' into multi-select-support-disabled-options
arjunlalb Jul 9, 2021
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
9 changes: 7 additions & 2 deletions projects/components/src/multi-select/multi-select.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
flex-direction: column;

.divider {
padding: 0px 16px 4px 16px;
padding: 0 16px 4px 16px;
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove px ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

zeroes don't need px :)

Copy link
Contributor

Choose a reason for hiding this comment

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

so this is just a nit change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea. this is just a minor cleanup since I was in the area :)

}

.multi-select-options {
Expand All @@ -107,8 +107,13 @@
font-size: 15px;
align-items: center;

&.disabled {
color: $gray-5;
cursor: not-allowed;
}

.checkbox {
margin: 0px;
margin: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

same question here

color: $gray-3;

::ng-deep .mat-ripple {
Expand Down
42 changes: 42 additions & 0 deletions projects/components/src/multi-select/multi-select.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// tslint:disable: max-file-line-count

import { CommonModule } from '@angular/common';
import { fakeAsync, flush } from '@angular/core/testing';
import { IconLibraryTestingModule, IconType } from '@hypertrace/assets-library';
Expand Down Expand Up @@ -476,4 +478,44 @@ describe('Multi Select Component', () => {
expect(spectator.query('.select-all', { root: true })).toExist();
flush();
}));

test('should disable select options as expected', fakeAsync(() => {
const onChange = jest.fn();

spectator = hostFactory(
`
<ht-multi-select (selectedChange)="onChange($event)">
<ht-select-option *ngFor="let option of options" [label]="option.label" [value]="option.value" [disabled]="option.disabled"></ht-select-option>
</ht-multi-select>`,
{
hostProps: {
options: [
{ label: 'first', value: 'first-value' },
{ label: 'second', value: 'second-value', disabled: true },
{
label: 'third',
value: 'third-value',
selectedLabel: 'Third Value!!!',
icon: 'test-icon',
iconColor: 'red'
}
],
onChange: onChange
}
}
);

spectator.tick();
spectator.click('.trigger-content');

const optionElements = spectator.queryAll('.multi-select-option', { root: true });
expect(optionElements.length).toBe(3);
expect(optionElements[0]).not.toHaveClass('disabled');
expect(optionElements[1]).toHaveClass('disabled');
expect(optionElements[2]).not.toHaveClass('disabled');
spectator.click(optionElements[1]);

expect(onChange).not.toHaveBeenCalled();
flush();
}));
});
6 changes: 6 additions & 0 deletions projects/components/src/multi-select/multi-select.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,13 @@ import { MultiSelectJustify } from './multi-select-justify';
*ngFor="let item of filteredOptions"
(click)="this.onSelectionChange(item)"
class="multi-select-option"
[ngClass]="{ disabled: item.disabled }"
>
<ht-checkbox
class="checkbox"
(click)="this.preventClickDefault($event)"
[checked]="this.isSelectedItem(item)"
[disabled]="item.disabled"
></ht-checkbox>
<ht-icon
class="icon"
Expand Down Expand Up @@ -207,6 +209,10 @@ export class MultiSelectComponent<V> implements AfterContentInit, OnChanges {
}

public onSelectionChange(item: SelectOptionComponent<V>): void {
if (item.disabled) {
return;
}

const selected = this.isSelectedItem(item)
? this.selected?.filter(value => value !== item.value)
: (this.selected ?? []).concat(item.value);
Expand Down