Skip to content

Commit 9efb374

Browse files
pierry01claudedjalmaaraujo
authored
fix(toast): initialize toaster state in initialize() so pre-existing targets don't throw (#499)
* fix(toast): init toaster state in initialize() to survive pre-existing targets Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Update toaster_controller.js Signed-off-by: Jean Pierry <pierrybm@gmail.com> * Update toaster_controller.js Signed-off-by: Jean Pierry <pierrybm@gmail.com> * chore(mcp): rebuild registry.json for toaster_controller fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(toast): clear _listEl on disconnect for symmetric teardown `connect()` resolves `_listEl`, but `disconnect()` left it pointing at the detached `<ol>`. Stimulus tears down in this order (Context#disconnect): `controller.disconnect()` first, then `targetObserver.stop()` -> `disconnectAllTargets()`, so every `toastTargetDisconnected` — and the `_reflow()` it triggers — runs *after* the controller disconnected, writing inline styles onto nodes on their way out. Null out `_listEl` at the end of `disconnect()` so `_reflow()`'s existing `if (!this._listEl) return` guard actually does its job. The global API closure (`window.RubyUI.toast`) outlives the controller and calls `_dismissById` / `_mutate` directly rather than through the removed window listeners, so those reads become optional and `_spawn` bails early instead of throwing on a null list. * refactor(toast): react to position/expand via value changed callbacks `position` and `expand` are Stimulus values, but nothing reacted to them changing: `_spawn` mirrored `positionValue` into `data-position` by hand, and `expand` was only read once in `connect()`. Flipping either attribute at runtime (`element.dataset.rubyUiToasterExpandValue = "true"`) had no effect until the next pointer event. Add `positionValueChanged` / `expandValueChanged` so the DOM follows the value: the position callback owns the `data-position` sync the CSS placement rules key off, and both reflow. `_spawn` now just assigns `positionValue`. Both callbacks fire during `valueObserver.start()`, before `connect()`, so they rely on `_reflow()`'s `if (!this._listEl) return` guard — same pattern as the target callbacks. `connect()` keeps `_expanded = this.expandValue`: `StringMapObserver#stop` does not clear its map, so an unchanged attribute fires no callback when the observer restarts. The region is `data-turbo-permanent`, so it really does disconnect/reconnect with the same controller instance, and without that line a toaster left hover-expanded would reconnect still expanded. * fix(toast): make toastTargetConnected idempotent Now that `_resizeObservers` lives in `initialize()` it survives disconnect/reconnect cycles of the same element, so a second `toastTargetConnected` for an element that still has an observer would overwrite the WeakMap entry and leak the first one — it keeps observing and keeps calling `_reflow()`. Stimulus guards against that today (`TargetObserver#connectTarget` skips elements already in `targetsByName`), but the callback should not depend on that internal: disconnect any existing observer for the element first. --------- Signed-off-by: Jean Pierry <pierrybm@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Djalma Araújo <djalma@nossomos.cc>
1 parent 58317ff commit 9efb374

3 files changed

Lines changed: 49 additions & 17 deletions

File tree

‎docs/app/javascript/controllers/ruby_ui/toaster_controller.js‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,13 @@ export default class extends Controller {
3939
dir: { type: String, default: "ltr" },
4040
}
4141

42-
connect() {
42+
initialize() {
43+
this._expanded = false
4344
this._heights = new Map()
4445
this._resizeObservers = new WeakMap()
46+
}
47+
48+
connect() {
4549
this._expanded = this.expandValue
4650
this._listEl = this.element.querySelector("ol") || (this.element.tagName === "OL" ? this.element : null)
4751
this._registerGlobalApi()
@@ -59,6 +63,8 @@ export default class extends Controller {
5963
this._listEl.addEventListener("pointerenter", this._onPointerEnter)
6064
this._listEl.addEventListener("pointerleave", this._onPointerLeave)
6165
document.addEventListener("keydown", this._onKey)
66+
67+
this._reflow()
6268
}
6369

6470
disconnect() {
@@ -67,9 +73,21 @@ export default class extends Controller {
6773
this._listEl?.removeEventListener("pointerenter", this._onPointerEnter)
6874
this._listEl?.removeEventListener("pointerleave", this._onPointerLeave)
6975
document.removeEventListener("keydown", this._onKey)
76+
this._listEl = null
77+
}
78+
79+
positionValueChanged(value) {
80+
this.element.setAttribute("data-position", value)
81+
this._reflow()
82+
}
83+
84+
expandValueChanged(value) {
85+
this._expanded = value
86+
this._reflow()
7087
}
7188

7289
toastTargetConnected(el) {
90+
this._resizeObservers.get(el)?.disconnect()
7391
if (typeof ResizeObserver !== "undefined") {
7492
const ro = new ResizeObserver(() => {
7593
this._heights.set(el, el.offsetHeight)
@@ -90,13 +108,11 @@ export default class extends Controller {
90108
}
91109

92110
_spawn(detail) {
111+
if (!this._listEl) return null
93112
const variant = VARIANTS.includes(detail.variant) ? detail.variant : "default"
94113
const tpl = this._skeletonFor(variant)
95114
if (!tpl) return null
96-
if (detail.position) {
97-
this.element.setAttribute("data-position", detail.position)
98-
this.positionValue = detail.position
99-
}
115+
if (detail.position) this.positionValue = detail.position
100116
const node = tpl.content.firstElementChild.cloneNode(true)
101117

102118
node.id = detail.id || `toast-${this._uuid()}`
@@ -151,7 +167,7 @@ export default class extends Controller {
151167
)
152168
return
153169
}
154-
const el = this._listEl.querySelector(`#${CSS.escape(id)}`)
170+
const el = this._listEl?.querySelector(`#${CSS.escape(id)}`)
155171
if (el) el.dispatchEvent(new CustomEvent("ruby-ui:toast:force-dismiss", { bubbles: true }))
156172
}
157173

@@ -243,7 +259,7 @@ export default class extends Controller {
243259
if (wantCtrl !== e.ctrlKey) return
244260
if (wantMeta !== e.metaKey) return
245261
e.preventDefault()
246-
const first = this._listEl.firstElementChild
262+
const first = this._listEl?.firstElementChild
247263
first?.focus()
248264
}
249265

@@ -278,7 +294,7 @@ export default class extends Controller {
278294
}
279295

280296
_mutate(id, variant, text) {
281-
const el = this._listEl.querySelector(`#${CSS.escape(id)}`)
297+
const el = this._listEl?.querySelector(`#${CSS.escape(id)}`)
282298
if (!el) return
283299
el.dataset.variant = variant
284300
el.setAttribute("role", variant === "error" ? "alert" : "status")

‎gem/lib/ruby_ui/toast/toaster_controller.js‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,13 @@ export default class extends Controller {
3939
dir: { type: String, default: "ltr" },
4040
}
4141

42-
connect() {
42+
initialize() {
43+
this._expanded = false
4344
this._heights = new Map()
4445
this._resizeObservers = new WeakMap()
46+
}
47+
48+
connect() {
4549
this._expanded = this.expandValue
4650
this._listEl = this.element.querySelector("ol") || (this.element.tagName === "OL" ? this.element : null)
4751
this._registerGlobalApi()
@@ -59,6 +63,8 @@ export default class extends Controller {
5963
this._listEl.addEventListener("pointerenter", this._onPointerEnter)
6064
this._listEl.addEventListener("pointerleave", this._onPointerLeave)
6165
document.addEventListener("keydown", this._onKey)
66+
67+
this._reflow()
6268
}
6369

6470
disconnect() {
@@ -67,9 +73,21 @@ export default class extends Controller {
6773
this._listEl?.removeEventListener("pointerenter", this._onPointerEnter)
6874
this._listEl?.removeEventListener("pointerleave", this._onPointerLeave)
6975
document.removeEventListener("keydown", this._onKey)
76+
this._listEl = null
77+
}
78+
79+
positionValueChanged(value) {
80+
this.element.setAttribute("data-position", value)
81+
this._reflow()
82+
}
83+
84+
expandValueChanged(value) {
85+
this._expanded = value
86+
this._reflow()
7087
}
7188

7289
toastTargetConnected(el) {
90+
this._resizeObservers.get(el)?.disconnect()
7391
if (typeof ResizeObserver !== "undefined") {
7492
const ro = new ResizeObserver(() => {
7593
this._heights.set(el, el.offsetHeight)
@@ -90,13 +108,11 @@ export default class extends Controller {
90108
}
91109

92110
_spawn(detail) {
111+
if (!this._listEl) return null
93112
const variant = VARIANTS.includes(detail.variant) ? detail.variant : "default"
94113
const tpl = this._skeletonFor(variant)
95114
if (!tpl) return null
96-
if (detail.position) {
97-
this.element.setAttribute("data-position", detail.position)
98-
this.positionValue = detail.position
99-
}
115+
if (detail.position) this.positionValue = detail.position
100116
const node = tpl.content.firstElementChild.cloneNode(true)
101117

102118
node.id = detail.id || `toast-${this._uuid()}`
@@ -151,7 +167,7 @@ export default class extends Controller {
151167
)
152168
return
153169
}
154-
const el = this._listEl.querySelector(`#${CSS.escape(id)}`)
170+
const el = this._listEl?.querySelector(`#${CSS.escape(id)}`)
155171
if (el) el.dispatchEvent(new CustomEvent("ruby-ui:toast:force-dismiss", { bubbles: true }))
156172
}
157173

@@ -243,7 +259,7 @@ export default class extends Controller {
243259
if (wantCtrl !== e.ctrlKey) return
244260
if (wantMeta !== e.metaKey) return
245261
e.preventDefault()
246-
const first = this._listEl.firstElementChild
262+
const first = this._listEl?.firstElementChild
247263
first?.focus()
248264
}
249265

@@ -278,7 +294,7 @@ export default class extends Controller {
278294
}
279295

280296
_mutate(id, variant, text) {
281-
const el = this._listEl.querySelector(`#${CSS.escape(id)}`)
297+
const el = this._listEl?.querySelector(`#${CSS.escape(id)}`)
282298
if (!el) return
283299
el.dataset.variant = variant
284300
el.setAttribute("role", variant === "error" ? "alert" : "status")

0 commit comments

Comments
 (0)