Skip to content

Comments

event triggers refactor - more generic eventTriggers, allow adding tr…#129

Open
michabeeri wants to merge 5 commits intomasterfrom
event-triggers-support.2
Open

event triggers refactor - more generic eventTriggers, allow adding tr…#129
michabeeri wants to merge 5 commits intomasterfrom
event-triggers-support.2

Conversation

@michabeeri
Copy link

…iggers more easily

Description

Related Issue

Checklist

  • I have read the Contributing Guide
  • I have added/updated tests for my changes (if applicable)
  • I have updated documentation/rules/skills (if applicable)

Screenshots / Demos

Additional Notes

Comment on lines +112 to +131
const focusListener = (e: FocusEvent) => {
if (!source.contains(e.relatedTarget as HTMLElement)) {
(handler as (e: FocusEvent) => void)(e);
}
};

const clickListener = (e: MouseEvent) => {
if ((e as PointerEvent).pointerType) {
(handler as (e: MouseEvent) => void)(e);
}
};

const keydownListener = (e: KeyboardEvent) => {
if (e.code === 'Space') {
e.preventDefault();
(handler as (e: KeyboardEvent) => void)(e);
} else if (e.code === 'Enter') {
(handler as (e: KeyboardEvent) => void)(e);
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want to move these outside and create them on-demand from a map by type.
The focusHandler should be created when the event type is focusin or focusout.
The clickHandler should be created when the event type is click, for now, we may extend this for other types.
The keydownListener can be the "default activate" handler, and should be created when event type is keydown. We'll probably extend these later for allowing other keys etc.

Comment on lines +143 to +144
const isEnterLeave = isEnterLeaveMode(genericConfig);
if (isEnterLeave) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be enough:

Suggested change
const isEnterLeave = isEnterLeaveMode(genericConfig);
if (isEnterLeave) {
if (enterLeave) {

Comment on lines +145 to +146
const enter = genericConfig.enter ?? [];
const leaveEvents = genericConfig.leave ?? [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are already guaranteed to be arrays at this point.

Comment on lines +114 to +115
const enterEvents: string[] = enterLeave?.enter ? [...enterLeave.enter] : [];
const leaveEvents: string[] = enterLeave?.leave ? [...enterLeave.leave] : [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not necessary, these are already converted to Arrays.

}
if (!isToggleMode && isLeave && isToggle) {
targetController.toggleEffect(effectId, 'remove', item);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
} else {
if (isEnter) {
targetController.toggleEffect(effectId, isToggle ? 'add' : method, item);
}
if (isLeave && isToggle) {
targetController.toggleEffect(effectId, 'remove', item);
}
}

}
}
} else {
animation.progress(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to add:

Suggested change
animation.progress(0);
animation.progress(0);
delete element.dataset.interactEnter;

if (animation.isCSS) {
animation.onFinish(() => {
fastdom.mutate(() => {
element.dataset[enterLeave ? 'motionEnter' : 'interactEnter'] = 'done';
Copy link
Collaborator

Choose a reason for hiding this comment

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

That was left by mistake:

Suggested change
element.dataset[enterLeave ? 'motionEnter' : 'interactEnter'] = 'done';
element.dataset.interactEnter = 'done';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants