Skip to content

Commit 0fb455f

Browse files
tbondwilkinsonAndrewKushnir
authored andcommitted
refactor(core): Move global dispatch behavior into Dispatcher. (angular#55692)
This behavior is now implemented by calling `dispatch` whether or not the `action` is populated. The `Dispatcher` then does global dispatch and early returns if there's no action. PR Close angular#55692
1 parent 67bb310 commit 0fb455f

File tree

5 files changed

+111
-111
lines changed

5 files changed

+111
-111
lines changed

packages/core/primitives/event-dispatch/src/base_dispatcher.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ export class BaseDispatcher {
3939
* @param dispatchDelegate A function that should handle dispatching an `EventInfoWrapper` to handlers.
4040
*/
4141
constructor(
42-
private readonly dispatchDelegate: (
43-
eventInfoWrapper: EventInfoWrapper,
44-
isGlobalDispatch?: boolean,
45-
) => void,
42+
private readonly dispatchDelegate: (eventInfoWrapper: EventInfoWrapper) => void,
4643
{eventReplayer = undefined}: {eventReplayer?: Replayer} = {},
4744
) {
4845
this.eventReplayer = eventReplayer;
@@ -67,20 +64,18 @@ export class BaseDispatcher {
6764
*
6865
* @param eventInfo The info for the event that triggered this call or the
6966
* queue of events from EventContract.
70-
* @param isGlobalDispatch If true, dispatches a global event instead of a
71-
* regular jsaction handler.
7267
*/
73-
dispatch(eventInfo: EventInfo, isGlobalDispatch?: boolean): void {
68+
dispatch(eventInfo: EventInfo): void {
7469
const eventInfoWrapper = new EventInfoWrapper(eventInfo);
7570
if (eventInfoWrapper.getIsReplay()) {
76-
if (isGlobalDispatch || !this.eventReplayer) {
71+
if (!this.eventReplayer) {
7772
return;
7873
}
7974
this.queueEventInfoWrapper(eventInfoWrapper);
8075
this.scheduleEventReplay();
8176
return;
8277
}
83-
this.dispatchDelegate(eventInfoWrapper, isGlobalDispatch);
78+
this.dispatchDelegate(eventInfoWrapper);
8479
}
8580

8681
/** Queue an `EventInfoWrapper` for replay. */
@@ -117,7 +112,7 @@ export function registerDispatcher(
117112
eventContract: UnrenamedEventContract,
118113
dispatcher: BaseDispatcher,
119114
) {
120-
eventContract.ecrd((eventInfo: EventInfo, globalDispatch?: boolean) => {
121-
dispatcher.dispatch(eventInfo, globalDispatch);
115+
eventContract.ecrd((eventInfo: EventInfo) => {
116+
dispatcher.dispatch(eventInfo);
122117
}, Restriction.I_AM_THE_JSACTION_FRAMEWORK);
123118
}

packages/core/primitives/event-dispatch/src/dispatcher.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ export class Dispatcher {
7171
) {
7272
this.eventReplayer = eventReplayer;
7373
this.baseDispatcher = new BaseDispatcher(
74-
(eventInfoWrapper: EventInfoWrapper, isGlobalDispatch?: boolean) => {
75-
this.dispatchToHandler(eventInfoWrapper, isGlobalDispatch);
74+
(eventInfoWrapper: EventInfoWrapper) => {
75+
this.dispatchToHandler(eventInfoWrapper);
7676
},
7777
{
7878
eventReplayer: (eventInfoWrappers) => {
@@ -102,42 +102,50 @@ export class Dispatcher {
102102
*
103103
* @param eventInfo The info for the event that triggered this call or the
104104
* queue of events from EventContract.
105-
* @param isGlobalDispatch If true, dispatches a global event instead of a
106-
* regular jsaction handler.
107105
*/
108106
dispatch(eventInfo: EventInfo, isGlobalDispatch?: boolean): void {
109-
this.baseDispatcher.dispatch(eventInfo, isGlobalDispatch);
107+
this.baseDispatcher.dispatch(eventInfo);
110108
}
111109

112110
/**
113111
* Dispatches an `EventInfoWrapper`.
114112
*/
115-
private dispatchToHandler(eventInfoWrapper: EventInfoWrapper, isGlobalDispatch?: boolean) {
116-
if (isGlobalDispatch) {
113+
private dispatchToHandler(eventInfoWrapper: EventInfoWrapper) {
114+
if (this.globalHandlers.size) {
115+
const globalEventInfoWrapper = eventInfoWrapper.clone();
116+
117+
// In some cases, `populateAction` will rewrite `click` events to
118+
// `clickonly`. Revert back to a regular click, otherwise we won't be able
119+
// to execute global event handlers registered on click events.
120+
if (globalEventInfoWrapper.getEventType() === EventType.CLICKONLY) {
121+
globalEventInfoWrapper.setEventType(EventType.CLICK);
122+
}
117123
// Skip everything related to jsaction handlers, and execute the global
118124
// handlers.
119-
const ev = eventInfoWrapper.getEvent();
120-
const eventTypeHandlers = this.globalHandlers.get(eventInfoWrapper.getEventType());
125+
const event = globalEventInfoWrapper.getEvent();
126+
const eventTypeHandlers = this.globalHandlers.get(globalEventInfoWrapper.getEventType());
121127
let shouldPreventDefault = false;
122128
if (eventTypeHandlers) {
123129
for (const handler of eventTypeHandlers) {
124-
if (handler(ev) === false) {
130+
if (handler(event) === false) {
125131
shouldPreventDefault = true;
126132
}
127133
}
128134
}
129135
if (shouldPreventDefault) {
130-
eventLib.preventDefault(ev);
136+
eventLib.preventDefault(event);
131137
}
138+
}
139+
140+
const action = eventInfoWrapper.getAction();
141+
if (!action) {
132142
return;
133143
}
134144

135145
if (this.stopPropagation) {
136146
stopPropagation(eventInfoWrapper);
137147
}
138148

139-
const action = eventInfoWrapper.getAction()!;
140-
141149
let handler: EventInfoWrapperHandler | void = undefined;
142150
if (this.getHandler) {
143151
handler = this.getHandler(eventInfoWrapper);
@@ -307,7 +315,7 @@ export function stopPropagation(eventInfoWrapper: EventInfoWrapper) {
307315
* Dispatcher.
308316
*/
309317
export function registerDispatcher(eventContract: UnrenamedEventContract, dispatcher: Dispatcher) {
310-
eventContract.ecrd((eventInfo: EventInfo, globalDispatch?: boolean) => {
311-
dispatcher.dispatch(eventInfo, globalDispatch);
318+
eventContract.ecrd((eventInfo: EventInfo) => {
319+
dispatcher.dispatch(eventInfo);
312320
}, Restriction.I_AM_THE_JSACTION_FRAMEWORK);
313321
}

packages/core/primitives/event-dispatch/src/eventcontract.ts

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -163,29 +163,14 @@ export class EventContract implements UnrenamedEventContract {
163163
// All events are queued when the dispatcher isn't yet loaded.
164164
eventInfoLib.setIsReplay(eventInfo, true);
165165
this.queuedEventInfos?.push(eventInfo);
166-
}
167-
this.actionResolver.resolve(eventInfo);
168-
169-
if (!this.dispatcher) {
170166
return;
171167
}
172-
const globalEventInfo: eventInfoLib.EventInfo = eventInfoLib.cloneEventInfo(eventInfo);
173-
174-
// In some cases, `populateAction` will rewrite `click` events to
175-
// `clickonly`. Revert back to a regular click, otherwise we won't be able
176-
// to execute global event handlers registered on click events.
177-
if (eventInfoLib.getEventType(globalEventInfo) === EventType.CLICKONLY) {
178-
eventInfoLib.setEventType(globalEventInfo, EventType.CLICK);
179-
}
180-
181-
this.dispatcher(globalEventInfo, /* dispatch global event */ true);
182-
168+
this.actionResolver.resolve(eventInfo);
183169
const action = eventInfoLib.getAction(eventInfo);
184-
if (!action) {
185-
return;
186-
}
187-
if (shouldPreventDefaultBeforeDispatching(eventInfoLib.getActionElement(action), eventInfo)) {
188-
eventLib.preventDefault(eventInfoLib.getEvent(eventInfo));
170+
if (action) {
171+
if (shouldPreventDefaultBeforeDispatching(eventInfoLib.getActionElement(action), eventInfo)) {
172+
eventLib.preventDefault(eventInfoLib.getEvent(eventInfo));
173+
}
189174
}
190175

191176
this.dispatcher(eventInfo);

packages/core/primitives/event-dispatch/test/dispatcher_test.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
*/
88

99
import {Dispatcher, Replayer} from '../src/dispatcher';
10-
import {ActionInfo, createEventInfo, EventInfo, EventInfoWrapper} from '../src/event_info';
10+
import {
11+
ActionInfo,
12+
createEventInfo,
13+
EventInfo,
14+
EventInfoWrapper,
15+
unsetAction,
16+
} from '../src/event_info';
1117
import {createEvent} from '../src/replay';
1218

1319
function createMockClickEvent() {
@@ -137,7 +143,8 @@ describe('dispatcher test.ts', () => {
137143
await expectAsync(replayed).toBePending();
138144

139145
const eventInfo1 = createTestEventInfo({isReplay: true});
140-
dispatcher.dispatch(eventInfo1, /* globalDispatch= */ true);
146+
unsetAction(eventInfo1);
147+
dispatcher.dispatch(eventInfo1);
141148

142149
await expectAsync(replayed).toBePending();
143150
});
@@ -199,7 +206,9 @@ describe('dispatcher test.ts', () => {
199206
const dispatcher = new Dispatcher();
200207
dispatcher.registerGlobalHandler('click', handler);
201208

202-
dispatcher.dispatch(createTestEventInfo(), true);
209+
const eventInfo = createTestEventInfo();
210+
unsetAction(eventInfo);
211+
dispatcher.dispatch(eventInfo);
203212

204213
expect(handler).toHaveBeenCalledTimes(1);
205214
});
@@ -213,7 +222,8 @@ describe('dispatcher test.ts', () => {
213222
event: createEvent({type: 'mousedown'} as Event),
214223
eventType: 'mousedown',
215224
});
216-
dispatcher.dispatch(eventInfo, true);
225+
unsetAction(eventInfo);
226+
dispatcher.dispatch(eventInfo);
217227

218228
expect(handler).not.toHaveBeenCalled();
219229
});
@@ -234,7 +244,8 @@ describe('dispatcher test.ts', () => {
234244
container,
235245
action: {name: 'foo', element: targetElement},
236246
});
237-
dispatcher.dispatch(eventInfo, true);
247+
unsetAction(eventInfo);
248+
dispatcher.dispatch(eventInfo);
238249
});
239250
targetElement.addEventListener('click', targetHandler);
240251
const parentHandler = jasmine.createSpy('parentHandler');
@@ -263,7 +274,8 @@ describe('dispatcher test.ts', () => {
263274
container,
264275
action: {name: 'foo', element: targetElement},
265276
});
266-
dispatcher.dispatch(eventInfo, /* globalDispatch= */ true);
277+
unsetAction(eventInfo);
278+
dispatcher.dispatch(eventInfo);
267279
});
268280
targetElement.addEventListener('click', targetHandler);
269281
const parentHandler = jasmine.createSpy('parentHandler');

0 commit comments

Comments
 (0)