Skip to content

Commit 65df40e

Browse files
feat: convert components to use OnPush change detection strategy 2 (#3100)
Signed-off-by: rrothschild18 <raultonello18@gmail.com> Signed-off-by: Akshat Patel <38994122+Akshat55@users.noreply.github.com> Co-authored-by: Akshat Patel <38994122+Akshat55@users.noreply.github.com>
1 parent 1ab083e commit 65df40e

File tree

9 files changed

+88
-56
lines changed

9 files changed

+88
-56
lines changed

src/combobox/combobox.component.spec.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { Component } from "@angular/core";
1+
import { Component, Input } from "@angular/core";
22
import { TestBed } from "@angular/core/testing";
3-
import { By } from "@angular/platform-browser";
3+
import { By } from "@angular/platform-browser";
44

55
import { IconModule } from "../icon/index";
66
import { I18nModule } from "../i18n/index";
@@ -29,9 +29,9 @@ import { PlaceholderModule } from "./../placeholder/index";
2929
})
3030
class ComboboxTest {
3131
items = [
32-
{id: "1", content: "one", selected: false},
33-
{id: "2", content: "two", selected: false},
34-
{id: "3", content: "three", selected: false}
32+
{ id: "1", content: "one", selected: false },
33+
{ id: "2", content: "two", selected: false },
34+
{ id: "3", content: "three", selected: false }
3535
];
3636
type = "single";
3737
itemValueKey = undefined;
@@ -55,7 +55,7 @@ describe("Combo box", () => {
5555
UtilsModule,
5656
PlaceholderModule
5757
],
58-
providers: [ DropdownService ]
58+
providers: [DropdownService]
5959
});
6060
});
6161

@@ -182,9 +182,9 @@ describe("Combo box", () => {
182182
textInput.dispatchEvent(new Event("input"));
183183

184184
wrapper.items = [
185-
{id: "4", content: "four", selected: false},
186-
{id: "5", content: "five", selected: false},
187-
{id: "6", content: "six", selected: false}
185+
{ id: "4", content: "four", selected: false },
186+
{ id: "5", content: "five", selected: false },
187+
{ id: "6", content: "six", selected: false }
188188
];
189189

190190
fixture.detectChanges();

src/dropdown/dropdown.component.spec.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { Component } from "@angular/core";
1+
import { Component, Input } from "@angular/core";
22
import { TestBed } from "@angular/core/testing";
3-
import { By } from "@angular/platform-browser";
3+
import { By } from "@angular/platform-browser";
44

55
import { Dropdown } from "./dropdown.component";
66
import { DropdownList } from "./list/dropdown-list.component";
@@ -18,16 +18,18 @@ import { IconModule } from "../icon/index";
1818
<cds-dropdown
1919
placeholder="test"
2020
class="custom-class"
21+
[isOpen]="isOpen"
2122
(selected)="onSelect()"
2223
[(ngModel)]="model">
2324
<cds-dropdown-list [items]="items"></cds-dropdown-list>
2425
</cds-dropdown>`
2526
})
2627
class DropdownTest {
2728
model = null;
28-
items = [{content: "one", id: 0, selected: false}, {content: "two", id: 1, selected: false}];
29+
items = [{ content: "one", id: 0, selected: false }, { content: "two", id: 1, selected: false }];
2930
selected: ListItem;
30-
onSelect() {}
31+
@Input() isOpen = false;
32+
onSelect() { }
3133
}
3234

3335
describe("Dropdown", () => {
@@ -47,7 +49,7 @@ describe("Dropdown", () => {
4749
IconModule,
4850
UtilsModule
4951
],
50-
providers: [ DropdownService ]
52+
providers: [DropdownService]
5153
});
5254
});
5355

@@ -78,7 +80,7 @@ describe("Dropdown", () => {
7880
spyOn(wrapper, "onSelect");
7981
fixture.detectChanges();
8082
element = fixture.debugElement.query(By.css("cds-dropdown"));
81-
element.componentInstance.menuIsClosed = false;
83+
fixture.componentRef.setInput("isOpen", true);
8284
fixture.detectChanges();
8385
element.nativeElement.querySelector(".cds--list-box__menu-item__option").click();
8486
fixture.detectChanges();

src/dropdown/dropdown.component.ts

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import {
1212
OnDestroy,
1313
HostBinding,
1414
TemplateRef,
15-
AfterViewInit
15+
AfterViewInit,
16+
ChangeDetectionStrategy,
17+
ChangeDetectorRef
1618
} from "@angular/core";
1719
import { NG_VALUE_ACCESSOR, ControlValueAccessor } from "@angular/forms";
1820

@@ -86,7 +88,7 @@ import { hasScrollableParents } from "carbon-components-angular/utils";
8688
'cds--dropdown--sm cds--list-box--sm': size === 'sm',
8789
'cds--dropdown--md cds--list-box--md': size === 'md',
8890
'cds--dropdown--lg cds--list-box--lg': size === 'lg',
89-
'cds--list-box--expanded': !menuIsClosed,
91+
'cds--list-box--expanded': isOpen,
9092
'cds--list-box--invalid': invalid
9193
}"
9294
[attr.data-invalid]="invalid ? true : null">
@@ -98,8 +100,8 @@ import { hasScrollableParents } from "carbon-components-angular/utils";
98100
[id]="id"
99101
type="button"
100102
class="cds--list-box__field"
101-
[ngClass]="{'a': !menuIsClosed}"
102-
[attr.aria-expanded]="!menuIsClosed"
103+
[ngClass]="{'a': isOpen}"
104+
[attr.aria-expanded]="isOpen"
103105
[attr.aria-disabled]="disabled"
104106
[attr.aria-readonly]="readonly"
105107
aria-haspopup="listbox"
@@ -141,7 +143,7 @@ import { hasScrollableParents } from "carbon-components-angular/utils";
141143
cdsIcon="chevron--down"
142144
size="16"
143145
[attr.aria-label]="menuButtonLabel"
144-
[ngClass]="{'cds--list-box__menu-icon--open': !menuIsClosed }">
146+
[ngClass]="{'cds--list-box__menu-icon--open': isOpen }">
145147
</svg>
146148
}
147149
</span>
@@ -164,7 +166,7 @@ import { hasScrollableParents } from "carbon-components-angular/utils";
164166
[ngClass]="{
165167
'cds--list-box--up': this.dropUp !== null && this.dropUp !== undefined ? dropUp : _dropUp
166168
}">
167-
@if (!menuIsClosed) {
169+
@if (isOpen) {
168170
<ng-content />
169171
}
170172
</div>
@@ -208,7 +210,8 @@ import { hasScrollableParents } from "carbon-components-angular/utils";
208210
useExisting: Dropdown,
209211
multi: true
210212
}
211-
]
213+
],
214+
changeDetection: ChangeDetectionStrategy.OnPush
212215
})
213216
export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDestroy, ControlValueAccessor {
214217
static dropdownCount = 0;
@@ -217,7 +220,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
217220
}
218221

219222
@HostBinding("class.cds--list-box__wrapper--fluid--focus") get fluidFocusClass() {
220-
return this.fluid && this._isFocused && this.menuIsClosed;
223+
return this.fluid && this._isFocused && !this.isOpen;
221224
}
222225

223226
protected get writtenValue() {
@@ -299,8 +302,8 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
299302
*/
300303
@Input() invalidText: string | TemplateRef<any>;
301304
/**
302-
* Set to `true` to show a warning (contents set by warningText)
303-
*/
305+
* Set to `true` to show a warning (contents set by warningText)
306+
*/
304307
@Input() warn = false;
305308
/**
306309
* Sets the warning text
@@ -377,7 +380,13 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
377380
/**
378381
* Set to `true` if the dropdown is closed (not expanded).
379382
*/
380-
menuIsClosed = true;
383+
@Input() isOpen = false;
384+
385+
/** Deprecated as of v6 - Will be removed in v7 */
386+
set menuIsClosed(isClosed: boolean) {
387+
this.isOpen = !isClosed;
388+
this.changeDetectorRef.markForCheck();
389+
}
381390

382391
/**
383392
* controls whether the `drop-up` class is applied
@@ -407,7 +416,9 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
407416
protected elementRef: ElementRef,
408417
protected i18n: I18n,
409418
protected dropdownService: DropdownService,
410-
protected elementService: ElementService) {}
419+
protected elementService: ElementService,
420+
protected changeDetectorRef: ChangeDetectorRef
421+
) {}
411422

412423
/**
413424
* Updates the `type` property in the `@ContentChild`.
@@ -550,7 +561,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
550561
/**
551562
* function passed in by `registerOnChange`
552563
*/
553-
propagateChange = (_: any) => { };
564+
propagateChange = (_: any) => {};
554565

555566
/**
556567
* `ControlValueAccessor` method to programmatically disable the dropdown.
@@ -572,32 +583,32 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
572583
return;
573584
}
574585

575-
if ((event.key === "Escape") && !this.menuIsClosed) {
586+
if ((event.key === "Escape") && this.isOpen) {
576587
event.stopImmediatePropagation(); // don't unintentionally close other widgets that listen for Escape
577588
}
578589
if (event.key === "Escape") {
579590
event.preventDefault();
580591
this.closeMenu();
581592
this.dropdownButton.nativeElement.focus();
582-
} else if (this.menuIsClosed && (event.key === " " || event.key === "ArrowDown" || event.key === "ArrowUp")) {
593+
} else if (!this.isOpen && (event.key === " " || event.key === "ArrowDown" || event.key === "ArrowUp")) {
583594
if (this.disableArrowKeys && (event.key === "ArrowDown" || event.key === "ArrowUp")) {
584595
return;
585596
}
586597
event.preventDefault();
587598
this.openMenu();
588599
}
589600

590-
if (!this.menuIsClosed && event.key === "Tab" && this.dropdownMenu.nativeElement.contains(event.target as Node)) {
601+
if (this.isOpen && event.key === "Tab" && this.dropdownMenu.nativeElement.contains(event.target as Node)) {
591602
this.closeMenu();
592603
}
593604

594-
if (!this.menuIsClosed && event.key === "Tab" && event.shiftKey) {
605+
if (this.isOpen && event.key === "Tab" && event.shiftKey) {
595606
this.closeMenu();
596607
}
597608

598609
if (this.type === "multi") { return; }
599610

600-
if (this.menuIsClosed) {
611+
if (!this.isOpen) {
601612
this.closedDropdownNavigation(event);
602613
}
603614
}
@@ -681,7 +692,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
681692
return false;
682693
}
683694

684-
_noop() { }
695+
_noop() {}
685696
/**
686697
* Handles clicks outside of the `Dropdown`.
687698
*/
@@ -694,22 +705,22 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
694705
}
695706
}
696707
_outsideKey(event) {
697-
if (!this.menuIsClosed && event.key === "Tab" && this.dropdownMenu.nativeElement.contains(event.target as Node)) {
708+
if (this.isOpen && event.key === "Tab" && this.dropdownMenu.nativeElement.contains(event.target as Node)) {
698709
this.closeMenu();
699710
}
700711
}
701712
/**
702713
* Handles keyboard events so users are controlling the `Dropdown` instead of unintentionally controlling outside elements.
703714
*/
704715
_keyboardNav(event: KeyboardEvent) {
705-
if (event.key === "Escape" && !this.menuIsClosed) {
716+
if (event.key === "Escape" && this.isOpen) {
706717
event.stopImmediatePropagation(); // don't unintentionally close modal if inside of it
707718
}
708719
if (event.key === "Escape") {
709720
event.preventDefault();
710721
this.closeMenu();
711722
this.dropdownButton.nativeElement.focus();
712-
} else if (!this.menuIsClosed && event.key === "Tab") {
723+
} else if (this.isOpen && event.key === "Tab") {
713724
// this way focus will start on the next focusable item from the dropdown
714725
// not the top of the body!
715726
this.dropdownButton.nativeElement.focus();
@@ -731,7 +742,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
731742
*/
732743
_appendToBody() {
733744
const lightClass = this.theme === "light" ? " cds--list-box--light" : "";
734-
const expandedClass = !this.menuIsClosed ? " cds--list-box--expanded" : "";
745+
const expandedClass = this.isOpen ? " cds--list-box--expanded" : "";
735746
this.dropdownService.appendToBody(
736747
this.dropdownButton.nativeElement,
737748
this.dropdownMenu.nativeElement,
@@ -770,7 +781,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
770781
}
771782

772783
this._dropUp = false;
773-
this.menuIsClosed = false;
784+
this.isOpen = true;
774785

775786
// move the dropdown list to the body if we're not appending inline
776787
// and position it relative to the dropdown wrapper
@@ -809,8 +820,8 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
809820
*/
810821
closeMenu() {
811822
// return early if the menu is already closed
812-
if (this.menuIsClosed) { return; }
813-
this.menuIsClosed = true;
823+
if (!this.isOpen) { return; }
824+
this.isOpen = false;
814825
this.checkForReorder();
815826
this.onClose.emit();
816827
this.close.emit();
@@ -838,7 +849,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
838849
* Controls toggling menu states between open/expanded and closed/collapsed.
839850
*/
840851
toggleMenu() {
841-
if (this.menuIsClosed) {
852+
if (!this.isOpen) {
842853
this.openMenu();
843854
} else {
844855
this.closeMenu();
@@ -860,7 +871,7 @@ export class Dropdown implements OnInit, AfterContentInit, AfterViewInit, OnDest
860871
* Controls when it's needed to apply the selection feedback
861872
*/
862873
protected checkForReorder() {
863-
const topAfterReopen = this.menuIsClosed && this.selectionFeedback === "top-after-reopen";
874+
const topAfterReopen = !this.isOpen && this.selectionFeedback === "top-after-reopen";
864875
if ((this.type === "multi") && (topAfterReopen || this.selectionFeedback === "top")) {
865876
this.view.reorderSelected();
866877
}

src/dropdown/dropdown.stories.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ export default {
6666
dropUp: false,
6767
size: "md",
6868
theme: "dark",
69-
fluid: false
69+
fluid: false,
70+
isOpen: false
7071
},
7172
argTypes: {
7273
type: {
@@ -105,6 +106,7 @@ const Template = (args) => ({
105106
[disabled]="disabled"
106107
[readonly]="readonly"
107108
[fluid]="fluid"
109+
[isOpen]="isOpen"
108110
(selected)="selected($event)"
109111
(onClose)="onClose($event)">
110112
<cds-dropdown-list [items]="items"></cds-dropdown-list>
@@ -139,6 +141,7 @@ const MultiTemplate = (args) => ({
139141
[disabled]="disabled"
140142
[readonly]="readonly"
141143
[fluid]="fluid"
144+
[isOpen]="isOpen"
142145
(selected)="selected($event)"
143146
(onClose)="onClose($event)">
144147
<cds-dropdown-list [items]="items"></cds-dropdown-list>

0 commit comments

Comments
 (0)