Skip to content

Commit 016ca44

Browse files
authored
Refactor disabled getter (remove ember/runloop & fix no-side-effects) (#1064)
* Refactor disabled getter (remove ember/runloop & fix no-side-effects) * Fix lint * Fix lint * Add note to migrate * Remove unwanted change
1 parent 7fac6a1 commit 016ca44

7 files changed

Lines changed: 80 additions & 26 deletions

File tree

docs/app/templates/public-pages/docs/migrate-8-0-to-9-0.gts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,18 @@ import { LinkTo } from '@ember/routing';
7777
a regular HTML attribute instead.
7878
</p>
7979
</li>
80+
<li>
81+
<p>
82+
If you are using the public API without the trigger modifier and change
83+
<code>@disabled</code>
84+
from
85+
<code>false</code>
86+
to
87+
<code>true</code>, please follow the guidance
88+
<a href="https://github.com/cibernox/ember-basic-dropdown/pull/1064">in
89+
this PR</a>
90+
</p>
91+
</li>
8092
<li>
8193
<p>
8294
If you are using

src/components/basic-dropdown-content.gts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ export default class BasicDropdownContent<
175175
if (!triggerElement) {
176176
triggerElement = document.querySelector(selector) as HTMLElement;
177177
}
178-
this.handleRootMouseDown = (e: MouseEvent | TouchEvent) => {
178+
this.handleRootMouseDown = (e: MouseEvent | TouchEvent): void => {
179179
const target = (e.composedPath?.()[0] || e.target) as Element;
180180
if (target === null) return;
181181
if (

src/components/basic-dropdown.gts

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { tracked } from '@glimmer/tracking';
33
import { action } from '@ember/object';
44
import { guidFor } from '@ember/object/internals';
55
import calculatePosition from '../utils/calculate-position.ts';
6-
import { schedule } from '@ember/runloop';
76
import { hash } from '@ember/helper';
87
import BasicDropdownTrigger from './basic-dropdown-trigger.gts';
98
import BasicDropdownContent from './basic-dropdown-content.gts';
@@ -27,7 +26,6 @@ import type {
2726
TRootEventType,
2827
} from '../types.ts';
2928

30-
const UNINITIALIZED = {};
3129
const IGNORED_STYLES = ['top', 'left', 'right', 'width', 'height'];
3230

3331
export interface BasicDropdownDefaultBlock<
@@ -122,35 +120,44 @@ export default class BasicDropdown<
122120
@tracked width: string | undefined;
123121
@tracked height: string | undefined;
124122
@tracked otherStyles: Record<string, string | number | undefined> = {};
125-
@tracked isOpen = this.args.initiallyOpened || false;
126123
@tracked renderInPlace =
127124
this.args.renderInPlace !== undefined ? this.args.renderInPlace : false;
125+
@tracked private _isOpen = this.args.initiallyOpened || false;
126+
128127
private previousVerticalPosition?: VerticalPosition | undefined;
129128
private previousHorizontalPosition?: HorizontalPosition | undefined;
130129
private triggerElement: HTMLElement | null = null;
131130
private dropdownElement: HTMLElement | null = null;
132131

133132
private _uid = guidFor(this);
134133
private _dropdownId: string = `ember-basic-dropdown-content-${this._uid}`;
135-
private _previousDisabled = UNINITIALIZED;
136134
private _actions: DropdownActions = {
137135
open: this.open.bind(this),
138136
close: this.close.bind(this),
139137
toggle: this.toggle.bind(this),
140138
reposition: this.reposition.bind(this),
139+
updatePublicApi: this.updatePublicApi.bind(this),
141140
registerTriggerElement: this.registerTriggerElement.bind(this),
142141
registerDropdownElement: this.registerDropdownElement.bind(this),
143142
getTriggerElement: () => this.triggerElement,
144143
};
145144

146-
private get horizontalPosition() {
145+
private get horizontalPosition(): HorizontalPosition {
147146
return this.args.horizontalPosition || 'auto'; // auto-right | right | center | left
148147
}
149148

150-
private get verticalPosition() {
149+
private get verticalPosition(): VerticalPosition {
151150
return this.args.verticalPosition || 'auto'; // above | below
152151
}
153152

153+
get isOpen(): boolean {
154+
return !this.disabled && this._isOpen;
155+
}
156+
157+
set isOpen(v: boolean) {
158+
this._isOpen = v;
159+
}
160+
154161
get destination(): string {
155162
return this.args.destination || this._getDestinationId();
156163
}
@@ -179,25 +186,7 @@ export default class BasicDropdown<
179186
}
180187

181188
get disabled(): boolean {
182-
const newVal = this.args.disabled || false;
183-
if (
184-
this._previousDisabled !== UNINITIALIZED &&
185-
this._previousDisabled !== newVal
186-
) {
187-
// eslint-disable-next-line ember/no-runloop
188-
schedule('actions', () => {
189-
if (newVal && this.publicAPI.isOpen) {
190-
// eslint-disable-next-line ember/no-side-effects
191-
this.isOpen = false;
192-
}
193-
if (this.args.registerAPI) {
194-
this.args.registerAPI(this.publicAPI);
195-
}
196-
});
197-
}
198-
// eslint-disable-next-line ember/no-side-effects
199-
this._previousDisabled = newVal;
200-
return newVal;
189+
return this.args.disabled || false;
201190
}
202191

203192
get publicAPI(): Dropdown {
@@ -337,6 +326,15 @@ export default class BasicDropdown<
337326
return this.applyReposition(triggerElement, dropdownElement, positionData);
338327
}
339328

329+
@action
330+
updatePublicApi() {
331+
if (!this.args.registerAPI) {
332+
return;
333+
}
334+
335+
this.args.registerAPI(this.publicAPI);
336+
}
337+
340338
@action
341339
registerTriggerElement(element: HTMLElement): void {
342340
this.triggerElement = element;

src/modifiers/basic-dropdown-trigger.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export default class DropdownTriggerModifier extends Modifier<Signature> {
3232
desiredEventType!: string;
3333
stopPropagation: boolean | undefined;
3434

35+
private _previousDisabled: boolean | undefined = undefined;
36+
3537
constructor(owner: Owner, args: ArgsFor<Signature>) {
3638
super(owner, args);
3739
registerDestructor(this, cleanup);
@@ -51,6 +53,17 @@ export default class DropdownTriggerModifier extends Modifier<Signature> {
5153
this.setup(element);
5254
this.didSetup = true;
5355
}
56+
57+
if (this._previousDisabled === undefined) {
58+
this._previousDisabled = this.dropdown.disabled;
59+
}
60+
61+
if (this.dropdown.disabled !== this._previousDisabled) {
62+
this._previousDisabled = this.dropdown.disabled;
63+
64+
this.dropdown.actions.updatePublicApi();
65+
}
66+
5467
this.update(element, positional, named);
5568
}
5669

src/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ export interface DropdownActions {
33
close: (e?: Event, skipFocus?: boolean) => void;
44
open: (e?: Event) => void;
55
reposition: () => undefined | RepositionChanges;
6+
updatePublicApi: () => void;
67
registerTriggerElement: (e: HTMLElement) => void;
78
registerDropdownElement: (e: HTMLElement) => void;
89
getTriggerElement: () => HTMLElement | null;

0 commit comments

Comments
 (0)