From 4b1240ca84e9a04b65b93ea5c46237f81eaf9c70 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 6 Dec 2022 21:00:55 -0500 Subject: [PATCH] Action Descriptor Syntax: Support Outlet listeners The original [idea][] for this change was outlined in a comment on [hotwired/stimulus#576]. The problem --- Prior to this commit, any Outlet-powered references would need to manage event listeners from within the Stimulus Controller JavaScript. For example, consider the following HTML: ```html This dialog is managed through a disclosure button powered by an Outlet.
``` Clicking the `button[type="button"]` opens the `dialog` element by calling its [HTMLDialogElement.showModal()][] method. Consider the following `element` and `disclosure` controller implementations: ```js // element_controller.js import { Controller } from "@hotwired/stimulus" export default class extends Controller { showModal() { this.element.showModal() } } // disclosure_controller.js import { Controller } from "@hotwired/stimulus" export default class extends Controller { static outlets = ["element"] elementOutletConnected(controller, element) { this.element.setAttribute("aria-controls", element.id) this.element.setAttribute("aria-expanded", element.open) element.addEventListener("close", this.collapse) } elementOutletDisconnected() { element.removeEventListener("close", this.collapse) this.element.removeAttribute("aria-controls") this.element.removeAttribute("aria-expanded") } collapse = () => { this.element.setAttribute("aria-expanded", false) this.element.focus() } expand() { for (const elementOutlet of this.elementOutlets) { elementOutlet.showModal() this.element.setAttribute("aria-expanded", elementOutlet.element.open) } } } ``` Note the mirrored calls to add and remove [close][] event listeners. Whenever the `dialog` element closes, it'll dispatch a `close` event, which the `disclosure` controller will want to respond to. Attaching and removing event listeners whenever an element connects or disconnects is one of Stimulus's core capabilities, and declaring event listeners as part of `[data-action]` is idiomatic. In spite of those facts, the `disclosure` controller is responsible for the tedium of managing its own event listeners. The proposal --- To push those declarations out of the JavaScript and back into the HTML, this commit extends the Action Descriptor syntax to support declaring actions with `@`-prefixed controller identifiers, in the same way that `window` and `document` are special-cased. With that support, the HTML changes: ```diff This dialog is managed through a disclosure button powered by an Outlet.
``` And our `disclosure` controller has fewer responsibilities, and doesn't need to special-case the `collapse` function's binding: ```diff elementOutletConnected(controller, element) { this.element.setAttribute("aria-controls", element.id) this.element.setAttribute("aria-expanded", element.open) - element.addEventListener("close", this.collapse) } elementOutletDisconnected() { - element.removeEventListener("close", this.collapse) this.element.removeAttribute("aria-controls") this.element.removeAttribute("aria-expanded") } - collapse = () => { + collapse() { this.element.setAttribute("aria-expanded", false) this.element.focus() } ``` Risks --- Changing the action descriptor syntax has more long-term maintenance risks that other implementation changes. If we "spend" the syntax on this type of support, we're pretty stuck with it. Similarly, existing support for `window` and `document` as special symbols means that we'd need to make special considerations (or warnings) to support applications with `window`- and `document`-identified controllers. [hotwired/stimulus#576]: https://github.com/hotwired/stimulus/pull/576 [idea]: https://github.com/hotwired/stimulus/pull/576#issuecomment-1336214087 [HTMLDialogElement.showModal()]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/showModal [close]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/close_event --- docs/reference/actions.md | 39 ++++++++-- docs/reference/outlets.md | 25 +++++++ examples/controllers/disclosure_controller.js | 27 +++++++ examples/controllers/element_controller.js | 7 ++ examples/index.js | 6 ++ examples/public/examples.css | 12 +++ examples/public/main.css | 1 + examples/server.js | 1 + examples/views/disclosures.ejs | 16 ++++ src/core/action.ts | 43 ++++++++--- src/core/action_descriptor.ts | 20 ++--- src/core/binding.ts | 4 +- src/core/binding_observer.ts | 7 +- src/core/context.ts | 1 + src/core/dispatcher.ts | 36 +++++---- src/core/outlet_observer.ts | 2 + src/tests/cases/dom_test_case.ts | 18 ++++- src/tests/controllers/element_controller.ts | 7 ++ src/tests/controllers/outlet_controller.ts | 2 +- .../modules/core/outlets/action_tests.ts | 75 +++++++++++++++++++ 20 files changed, 298 insertions(+), 51 deletions(-) create mode 100644 examples/controllers/disclosure_controller.js create mode 100644 examples/controllers/element_controller.js create mode 100644 examples/views/disclosures.ejs create mode 100644 src/tests/controllers/element_controller.ts create mode 100644 src/tests/modules/core/outlets/action_tests.ts diff --git a/docs/reference/actions.md b/docs/reference/actions.md index aebae6ab..829bd64c 100644 --- a/docs/reference/actions.md +++ b/docs/reference/actions.md @@ -125,11 +125,36 @@ The list of supported modifier keys is shown below. | `meta` | Command key on MacOS | | `shift` | | +### Outlet Events + +Sometimes a controller needs to listen for events dispatched on elements made available through its [Outlets](./outlets). + +You can append an [outlet controller's identifier](./outlets#attributes-and-names) prefixed by `@` (along with any filter modifier) in an action descriptor to install the event listener on that outlet's element, as in the following example: + + + +```html + + + + A modal dialog + +``` + +In this example, the ` ``` @@ -331,13 +356,13 @@ It will call both `ItemController#upvote` and `SpinnerController#start`, but onl // ItemController upvote(event) { // { id: 12345, url: "/votes", active: true, payload: { value: 1234567 } } - console.log(event.params) + console.log(event.params) } // SpinnerController start(event) { // {} - console.log(event.params) + console.log(event.params) } ``` @@ -346,7 +371,7 @@ If we don't need anything else from the event, we can destruct the params: ```js upvote({ params }) { // { id: 12345, url: "/votes", active: true, payload: { value: 1234567 } } - console.log(params) + console.log(params) } ``` diff --git a/docs/reference/outlets.md b/docs/reference/outlets.md index 8f19ad21..56c7974a 100644 --- a/docs/reference/outlets.md +++ b/docs/reference/outlets.md @@ -177,3 +177,28 @@ Would result in: Missing "data-controller=result" attribute on outlet element for "search" controller` ``` + +## Actions + +An element that declares outlets can listen for events dispatched on its outlets' elements. + +To attach an event listener whenever an associated outlet connects to the document, declare the host element's action descriptor with [outlet controller's identifier](./outlets#attributes-and-names) prefixed by `@` (along with any filter modifier), as in the following example: + + + +```html + + + + A modal dialog + +``` + +In this example, the ` + + + + + +<%- include("layout/tail") %> diff --git a/src/core/action.ts b/src/core/action.ts index 8fba7a10..dd009c64 100644 --- a/src/core/action.ts +++ b/src/core/action.ts @@ -1,37 +1,38 @@ import { ActionDescriptor, parseActionDescriptorString, stringifyEventTarget } from "./action_descriptor" import { Token } from "../mutation-observers" -import { Schema } from "./schema" +import { Context } from "./context" +import { Controller } from "./controller" import { camelize } from "./string_helpers" export class Action { readonly element: Element readonly index: number - readonly eventTarget: EventTarget + readonly eventTargets: EventTarget[] readonly eventName: string readonly eventOptions: AddEventListenerOptions readonly identifier: string readonly methodName: string readonly keyFilter: string - readonly schema: Schema + readonly context: Context - static forToken(token: Token, schema: Schema) { - return new this(token.element, token.index, parseActionDescriptorString(token.content), schema) + static forToken(token: Token, context: Context) { + return new this(token.element, token.index, parseActionDescriptorString(token.content), context) } - constructor(element: Element, index: number, descriptor: Partial, schema: Schema) { + constructor(element: Element, index: number, descriptor: Partial, context: Context) { this.element = element this.index = index - this.eventTarget = descriptor.eventTarget || element + this.eventTargets = parseEventTargets(context.controller, descriptor.eventTargets, element) this.eventName = descriptor.eventName || getDefaultEventNameForElement(element) || error("missing event name") this.eventOptions = descriptor.eventOptions || {} this.identifier = descriptor.identifier || error("missing identifier") this.methodName = descriptor.methodName || error("missing method name") this.keyFilter = descriptor.keyFilter || "" - this.schema = schema + this.context = context } toString() { const eventFilter = this.keyFilter ? `.${this.keyFilter}` : "" - const eventTarget = this.eventTargetName ? `@${this.eventTargetName}` : "" + const eventTarget = this.eventTargetNames ? `@${this.eventTargetNames}` : "" return `${this.eventName}${eventFilter}${eventTarget}->${this.identifier}#${this.methodName}` } @@ -75,8 +76,12 @@ export class Action { return params } - private get eventTargetName() { - return stringifyEventTarget(this.eventTarget) + get schema() { + return this.context.schema + } + + private get eventTargetNames() { + return stringifyEventTarget(this.eventTargets) } private get keyMappings() { @@ -112,3 +117,19 @@ function typecast(value: any): any { return value } } + +function parseEventTargets( + controller: Controller, + eventTargetName: string | undefined, + fallback: EventTarget +): EventTarget[] { + if (eventTargetName == "window") { + return [window] + } else if (eventTargetName == "document") { + return [document] + } else if (typeof eventTargetName == "string") { + return controller.outlets.findAll(eventTargetName) + } else { + return [fallback] + } +} diff --git a/src/core/action_descriptor.ts b/src/core/action_descriptor.ts index fe051981..391fb601 100644 --- a/src/core/action_descriptor.ts +++ b/src/core/action_descriptor.ts @@ -30,7 +30,7 @@ export const defaultActionDescriptorFilters: ActionDescriptorFilters = { } export interface ActionDescriptor { - eventTarget: EventTarget + eventTargets: string eventOptions: AddEventListenerOptions eventName: string identifier: string @@ -38,8 +38,8 @@ export interface ActionDescriptor { keyFilter: string } -// capture nos.: 1 1 2 2 3 3 4 4 5 5 6 6 -const descriptorPattern = /^(?:(.+?)(?:\.(.+?))?(?:@(window|document))?->)?(.+?)(?:#([^:]+?))(?::(.+))?$/ +// capture nos.: 1 1 2 2 3 3 4 4 5 5 6 6 +const descriptorPattern = /^(?:(.+?)(?:\.(.+?))?(?:@(.+?))?->)?(.+?)(?:#([^:]+?))(?::(.+))?$/ export function parseActionDescriptorString(descriptorString: string): Partial { const source = descriptorString.trim() @@ -53,7 +53,7 @@ export function parseActionDescriptorString(descriptorString: string): Partial Object.assign(options, { [token.replace(/^!/, "")]: !/^!/.test(token) }), {}) } -export function stringifyEventTarget(eventTarget: EventTarget) { +export function stringifyEventTarget(eventTargets: EventTarget[]) { + const [eventTarget] = eventTargets + if (eventTarget == window) { return "window" } else if (eventTarget == document) { diff --git a/src/core/binding.ts b/src/core/binding.ts index 2c3edc04..ed690cc3 100644 --- a/src/core/binding.ts +++ b/src/core/binding.ts @@ -16,8 +16,8 @@ export class Binding { return this.action.index } - get eventTarget(): EventTarget { - return this.action.eventTarget + get eventTargets(): EventTarget[] { + return this.action.eventTargets } get eventOptions(): AddEventListenerOptions { diff --git a/src/core/binding_observer.ts b/src/core/binding_observer.ts index 62cc8355..47c3282b 100644 --- a/src/core/binding_observer.ts +++ b/src/core/binding_observer.ts @@ -37,6 +37,11 @@ export class BindingObserver implements ValueListObserverDelegate { } } + refresh() { + this.stop() + this.start() + } + get element() { return this.context.element } @@ -79,7 +84,7 @@ export class BindingObserver implements ValueListObserverDelegate { // Value observer delegate parseValueForToken(token: Token): Action | undefined { - const action = Action.forToken(token, this.schema) + const action = Action.forToken(token, this.context) if (action.identifier == this.identifier) { return action } diff --git a/src/core/context.ts b/src/core/context.ts index e1187add..059346f2 100644 --- a/src/core/context.ts +++ b/src/core/context.ts @@ -53,6 +53,7 @@ export class Context implements ErrorHandler, TargetObserverDelegate, OutletObse refresh() { this.outletObserver.refresh() + this.bindingObserver.refresh() } disconnect() { diff --git a/src/core/dispatcher.ts b/src/core/dispatcher.ts index 04a3bb27..43d6dfac 100644 --- a/src/core/dispatcher.ts +++ b/src/core/dispatcher.ts @@ -38,11 +38,15 @@ export class Dispatcher implements BindingObserverDelegate { // Binding observer delegate bindingConnected(binding: Binding) { - this.fetchEventListenerForBinding(binding).bindingConnected(binding) + for (const eventListener of this.fetchEventListenersForBinding(binding)) { + eventListener.bindingConnected(binding) + } } bindingDisconnected(binding: Binding, clearEventListeners = false) { - this.fetchEventListenerForBinding(binding).bindingDisconnected(binding) + for (const eventListener of this.fetchEventListenersForBinding(binding)) { + eventListener.bindingDisconnected(binding) + } if (clearEventListeners) this.clearEventListenersForBinding(binding) } @@ -53,25 +57,29 @@ export class Dispatcher implements BindingObserverDelegate { } private clearEventListenersForBinding(binding: Binding) { - const eventListener = this.fetchEventListenerForBinding(binding) - if (!eventListener.hasBindings()) { - eventListener.disconnect() - this.removeMappedEventListenerFor(binding) + for (const eventListener of this.fetchEventListenersForBinding(binding)) { + if (!eventListener.hasBindings()) { + eventListener.disconnect() + this.removeMappedEventListenerFor(binding) + } } } private removeMappedEventListenerFor(binding: Binding) { - const { eventTarget, eventName, eventOptions } = binding - const eventListenerMap = this.fetchEventListenerMapForEventTarget(eventTarget) - const cacheKey = this.cacheKey(eventName, eventOptions) + const { eventTargets, eventName, eventOptions } = binding + + for (const eventTarget of eventTargets) { + const eventListenerMap = this.fetchEventListenerMapForEventTarget(eventTarget) + const cacheKey = this.cacheKey(eventName, eventOptions) - eventListenerMap.delete(cacheKey) - if (eventListenerMap.size == 0) this.eventListenerMaps.delete(eventTarget) + eventListenerMap.delete(cacheKey) + if (eventListenerMap.size == 0) this.eventListenerMaps.delete(eventTarget) + } } - private fetchEventListenerForBinding(binding: Binding): EventListener { - const { eventTarget, eventName, eventOptions } = binding - return this.fetchEventListener(eventTarget, eventName, eventOptions) + private fetchEventListenersForBinding(binding: Binding): EventListener[] { + const { eventTargets, eventName, eventOptions } = binding + return eventTargets.map((eventTarget) => this.fetchEventListener(eventTarget, eventName, eventOptions)) } private fetchEventListener( diff --git a/src/core/outlet_observer.ts b/src/core/outlet_observer.ts index 0208129f..ecc7f171 100644 --- a/src/core/outlet_observer.ts +++ b/src/core/outlet_observer.ts @@ -87,6 +87,7 @@ export class OutletObserver implements SelectorObserverDelegate { if (!this.outletElementsByName.has(outletName, element)) { this.outletsByName.add(outletName, outlet) this.outletElementsByName.add(outletName, element) + this.context.refresh() this.selectorObserverMap.get(outletName)?.pause(() => this.delegate.outletConnected(outlet, element, outletName)) } } @@ -95,6 +96,7 @@ export class OutletObserver implements SelectorObserverDelegate { if (this.outletElementsByName.has(outletName, element)) { this.outletsByName.delete(outletName, outlet) this.outletElementsByName.delete(outletName, element) + this.context.refresh() this.selectorObserverMap .get(outletName) ?.pause(() => this.delegate.outletDisconnected(outlet, element, outletName)) diff --git a/src/tests/cases/dom_test_case.ts b/src/tests/cases/dom_test_case.ts index 438239ef..fe86ad5b 100644 --- a/src/tests/cases/dom_test_case.ts +++ b/src/tests/cases/dom_test_case.ts @@ -60,8 +60,22 @@ export class DOMTestCase extends TestCase { return event } - findElement(selector: string) { - const element = this.fixtureElement.querySelector(selector) + async setAttribute(selectorOrElement: string | Element, name: string, value: string) { + const element = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement + + element.setAttribute(name, value) + await this.nextFrame + } + + async removeAttribute(selectorOrElement: string | Element, name: string) { + const element = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement + + element.removeAttribute(name) + await this.nextFrame + } + + findElement(selector: string) { + const element = this.fixtureElement.querySelector(selector) if (element) { return element } else { diff --git a/src/tests/controllers/element_controller.ts b/src/tests/controllers/element_controller.ts new file mode 100644 index 00000000..ceb64400 --- /dev/null +++ b/src/tests/controllers/element_controller.ts @@ -0,0 +1,7 @@ +import { Controller } from "../../core/controller" + +export class ElementController extends Controller { + click() { + this.element.click() + } +} diff --git a/src/tests/controllers/outlet_controller.ts b/src/tests/controllers/outlet_controller.ts index c3a5b840..8ee78dc5 100644 --- a/src/tests/controllers/outlet_controller.ts +++ b/src/tests/controllers/outlet_controller.ts @@ -12,7 +12,7 @@ class BaseOutletController extends Controller { export class OutletController extends BaseOutletController { static classes = ["connected", "disconnected"] - static outlets = ["beta", "gamma", "delta", "omega", "namespaced--epsilon"] + static outlets = ["beta", "gamma", "delta", "omega", "namespaced--epsilon", "element"] static values = { alphaOutletConnectedCallCount: Number, diff --git a/src/tests/modules/core/outlets/action_tests.ts b/src/tests/modules/core/outlets/action_tests.ts new file mode 100644 index 00000000..2344ec8e --- /dev/null +++ b/src/tests/modules/core/outlets/action_tests.ts @@ -0,0 +1,75 @@ +import { ApplicationTestCase } from "../../../cases/application_test_case" +import { ElementController } from "../../../controllers/element_controller" +import { LogController } from "../../../controllers/log_controller" +import { OutletController } from "../../../controllers/outlet_controller" + +export default class OutletsActionTests extends ApplicationTestCase { + fixtureHTML = ` + + + ` + + setupApplication() { + super.setupApplication() + this.application.register("element", ElementController) + this.application.register("log", LogController) + this.application.register("outlet", OutletController) + } + + async "test action descriptor with @-prefixed outlet-name attaches event listeners"() { + const outletButton = this.findElement("#outlet-button") + + await this.triggerEvent(outletButton, "click") + + const [action, ...rest] = this.actionLog + this.assert.ok(action) + this.assert.equal(action.name, "log") + this.assert.equal(action.identifier, "log") + this.assert.equal(action.eventType, "click") + this.assert.equal(action.currentTarget, outletButton) + this.assert.equal(rest.length, 0) + } + + async "test action descriptor with @-prefixed does not attach event listener to host element"() { + await this.triggerEvent("#controller-element", "click") + + this.assert.equal(this.actionLog.length, 0) + } + + async "test action descriptor with @-prefixed outlet-name attaches event listeners when connected"() { + const controllerElement = this.findElement("#controller-element") + + await this.removeAttribute(controllerElement, "data-outlet-element-outlet") + await this.setAttribute(controllerElement, "data-outlet-element-outlet", "#outlet-button") + await this.triggerEvent("#outlet-button", "click") + + this.assert.equal(this.actionLog.length, 0) + } + + async "test action descriptor with @-prefixed outlet-name removes event listeners when disconnected"() { + const controllerElement = this.findElement("#controller-element") + + await this.removeAttribute(controllerElement, "data-outlet-element-outlet") + await this.triggerEvent("#outlet-button", "click") + + this.assert.equal(this.actionLog.length, 0) + } + + get actionLog() { + const element = this.findElement(`[data-controller~="log"]`) + const controller = this.application.getControllerForElementAndIdentifier(element, "log") + + if (controller instanceof LogController) { + return controller.actionLog + } else { + throw new Error(`controller with identifier "log" must be instance of LogController`) + } + } +}