Skip to content
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.

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

Drop @ember/render-modifiers #714

Merged
merged 7 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 4 additions & 7 deletions addon/components/basic-dropdown-content.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@
@otherStyles
this.positionStyles
}}
{{did-insert this.setup}}
{{did-insert @dropdown.actions.reposition}}
{{did-insert this.setupMutationObserver}}
{{did-insert this.animateIn}}
{{will-destroy this.teardownMutationObserver}}
{{will-destroy this.animateOut}}
{{will-destroy this.teardown}}
{{this.respondToEvents}}
{{this.initiallyReposition}}
{{this.observeMutations}}
{{this.animateInAndOut}}
{{on 'focusin' (fn (or @onFocusIn this.noop) @dropdown)}}
{{on 'focusout' (fn (or @onFocusOut this.noop) @dropdown)}}
{{on 'mouseenter' (fn (or @onMouseEnter this.noop) @dropdown)}}
Expand Down
113 changes: 58 additions & 55 deletions addon/components/basic-dropdown-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import hasMoved from '../utils/has-moved';
import { Dropdown } from './basic-dropdown';
import { isTesting } from '@embroider/macros';
import { modifier } from 'ember-modifier';

interface Args {
animationEnabled?: boolean;
Expand Down Expand Up @@ -66,7 +67,7 @@ export default class BasicDropdownContent extends Component<Args> {

return animationEnabledArg && !isTesting();
}

get positionStyles(): Record<string, string> {
const style: Record<string, string> = {};
if (this.args.top !== undefined) {
Expand Down Expand Up @@ -97,8 +98,7 @@ export default class BasicDropdownContent extends Component<Args> {
*/
noop(): void {}

@action
setup(dropdownElement: Element): void {
respondToEvents = modifier((dropdownElement: Element): () => void => {
let triggerElement = document.querySelector(
`[data-ebd-id=${this.args.dropdown.uniqueId}-trigger]`
);
Expand Down Expand Up @@ -137,61 +137,65 @@ export default class BasicDropdownContent extends Component<Args> {
this.scrollableAncestors = getScrollableAncestors(triggerElement);
}
this.addScrollHandling(dropdownElement);
}

@action
teardown(): void {
this.removeGlobalEvents();
this.removeScrollHandling();
this.scrollableAncestors = [];

document.removeEventListener(
this.args.rootEventType,
this.handleRootMouseDown as RootMouseDownHandler,
true
);
return () => {
this.removeGlobalEvents();
this.removeScrollHandling();
this.scrollableAncestors = [];

if (this.isTouchDevice) {
document.removeEventListener('touchstart', this.touchStartHandler, true);
document.removeEventListener(
'touchend',
this.args.rootEventType,
this.handleRootMouseDown as RootMouseDownHandler,
true
);

if (this.isTouchDevice) {
document.removeEventListener('touchstart', this.touchStartHandler, true);
document.removeEventListener(
'touchend',
this.handleRootMouseDown as RootMouseDownHandler,
true
);
}
}
}
// @ts-ignore
}, { eager: false });
Copy link
Contributor Author

@gilest gilest Oct 23, 2023

Choose a reason for hiding this comment

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

All modifier() invocations must include { eager: false } since this package supports ember-modifier v3.2.x and v4.

For apps using v3.2.x { eager: false } will ensure the default autotracking behaviour (eager).

For apps using v4 this argument will be ignored.


@action
animateIn(dropdownElement: Element): void {
if (!this.animationEnabled) return;
waitForAnimations(dropdownElement, () => {
this.animationClass = this.transitionedInClass;
initiallyReposition = modifier(() => {
// Escape autotracking frame and avoid backtracking re-render
Promise.resolve().then(() => {
this.args.dropdown.actions.reposition()
Comment on lines +164 to +166
Copy link
Contributor Author

@gilest gilest Oct 23, 2023

Choose a reason for hiding this comment

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

This is the only code change I made. Since the did-insert modifier did not participate in auto tracking this was not an issue before.

I'm reasonably confident that this change is compatible since the tests pass. If this modifier doesn't run a number of tests will fail.

Someone more familiar with this library may have a better suggestion to replacing this operation with a derived data pattern (since this is a side-effect type modifier).

});
}
// @ts-ignore
}, { eager: false });

@action
animateOut(dropdownElement: Element): void {
if (!this.animationEnabled) return;
let parentElement =
dropdownElement.parentElement ?? this.destinationElement;
if (parentElement === null) return;
if (this.args.renderInPlace) {
parentElement = parentElement.parentElement;
}
if (parentElement === null) return;
let clone = dropdownElement.cloneNode(true) as Element;
clone.id = `${clone.id}--clone`;
clone.classList.remove(...this.transitioningInClass.split(' '));
clone.classList.add(...this.transitioningOutClass.split(' '));
parentElement.appendChild(clone);
this.animationClass = this.transitioningInClass;
waitForAnimations(clone, function () {
(parentElement as HTMLElement).removeChild(clone);
animateInAndOut = modifier((dropdownElement: Element): () => void => {
if (!this.animationEnabled) return () => {};
waitForAnimations(dropdownElement, () => {
this.animationClass = this.transitionedInClass;
});
}
return () => {
if (!this.animationEnabled) return;
let parentElement =
dropdownElement.parentElement ?? this.destinationElement;
if (parentElement === null) return;
if (this.args.renderInPlace) {
parentElement = parentElement.parentElement;
}
if (parentElement === null) return;
let clone = dropdownElement.cloneNode(true) as Element;
clone.id = `${clone.id}--clone`;
clone.classList.remove(...this.transitioningInClass.split(' '));
clone.classList.add(...this.transitioningOutClass.split(' '));
parentElement.appendChild(clone);
this.animationClass = this.transitioningInClass;
waitForAnimations(clone, function () {
(parentElement as HTMLElement).removeChild(clone);
});
};
// @ts-ignore
}, { eager: false });

@action
setupMutationObserver(dropdownElement: Element): void {
observeMutations = modifier((dropdownElement: Element): () => void => {
this.mutationObserver = new MutationObserver((mutations) => {
let shouldReposition = mutations.some(
(record: MutationRecord) =>
Expand All @@ -214,15 +218,14 @@ export default class BasicDropdownContent extends Component<Args> {
childList: true,
subtree: true,
});
}

@action
teardownMutationObserver(): void {
if (this.mutationObserver !== undefined) {
this.mutationObserver.disconnect();
this.mutationObserver = undefined;
return () => {
if (this.mutationObserver !== undefined) {
this.mutationObserver.disconnect();
this.mutationObserver = undefined;
}
}
}
// @ts-ignore
}, { eager: false });

@action
touchStartHandler(): void {
Expand Down
5 changes: 4 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
"test": "tests"
},
"dependencies": {
"@ember/render-modifiers": "^2.0.5",
"@embroider/macros": "^1.12.0",
"@embroider/util": "^1.11.0",
"@glimmer/component": "^1.1.2",
Expand Down
5 changes: 3 additions & 2 deletions tests/dummy/app/components/autofocus-input.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import Component from '@glimmer/component';
import { modifier } from 'ember-modifier';

export default class extends Component {
focusInput(input) {
focusInput = modifier((input) => {
input.focus();
}
});
}
2 changes: 1 addition & 1 deletion tests/dummy/app/templates/components/autofocus-input.hbs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<input type='text' {{did-insert this.focusInput}} ...attributes />
<input type='text' {{this.focusInput}} ...attributes />
Loading