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

Make Trix compatible with morphing #1227

Merged
merged 2 commits into from
Mar 10, 2025
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
13 changes: 0 additions & 13 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,12 @@ if (process.env.SAUCE_ACCESS_KEY) {
browserVersion: "latest",
"moz:debuggerAddress": true
},
sl_safari_12_1: {
base: "SauceLabs",
browserName: "safari",
platform: "macOS 10.13",
version: "12.1"
},
sl_edge_latest: {
base: "SauceLabs",
browserName: "microsoftedge",
platform: "Windows 10",
version: "latest"
},
sl_ios_latest: {
base: "SauceLabs",
browserName: "safari",
platform: "ios",
device: "iPhone X Simulator",
version: "13.0"
},
sl_android_9: {
base: "SauceLabs",
browserName: "chrome",
Expand Down
1 change: 1 addition & 0 deletions src/test/system.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import "test/system/html_replacement_test"
import "test/system/installation_process_test"
import "test/system/level_2_input_test"
import "test/system/list_formatting_test"
import "test/system/morphing_test"
import "test/system/mutation_input_test"
import "test/system/pasting_test"
import "test/system/text_formatting_test"
Expand Down
36 changes: 36 additions & 0 deletions src/test/system/morphing_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { assert, test, testGroup } from "test/test_helper"
import { nextFrame } from "../test_helpers/timing_helpers"

testGroup("morphing with internal toolbar", { template: "editor_empty" }, () => {
test("removing the 'connected' attribute will reset the editor and recreate toolbar", async () => {
const element = getEditorElement()

assert.ok(element.hasAttribute("connected"))

const originalToolbar = element.toolbarElement
element.toolbarElement.remove()
element.removeAttribute("toolbar")
element.removeAttribute("connected")
await nextFrame()

assert.ok(element.hasAttribute("connected"))
assert.ok(element.toolbarElement)
assert.notEqual(originalToolbar, element.toolbarElement)
})
})

testGroup("morphing with external toolbar", { template: "editor_with_toolbar_and_input" }, () => {
test("removing the 'connected' attribute will reset the editor leave the toolbar untouched", async () => {
const element = getEditorElement()

assert.ok(element.hasAttribute("connected"))

const originalToolbar = element.toolbarElement
element.removeAttribute("connected")
await nextFrame()

assert.ok(element.hasAttribute("connected"))
assert.ok(element.toolbarElement)
assert.equal(originalToolbar, element.toolbarElement)
})
})
30 changes: 27 additions & 3 deletions src/trix/elements/trix_editor_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ class LegacyDelegate {
export default class TrixEditorElement extends HTMLElement {
static formAssociated = "ElementInternals" in window

static observedAttributes = [ "connected" ]

#delegate

constructor() {
Expand Down Expand Up @@ -410,9 +412,9 @@ export default class TrixEditorElement extends HTMLElement {
} else if (this.parentNode) {
const toolbarId = `trix-toolbar-${this.trixId}`
this.setAttribute("toolbar", toolbarId)
const element = makeElement("trix-toolbar", { id: toolbarId })
this.parentNode.insertBefore(element, this)
return element
this.internalToolbar = makeElement("trix-toolbar", { id: toolbarId })
this.parentNode.insertBefore(this.internalToolbar, this)
return this.internalToolbar
} else {
return undefined
}
Expand Down Expand Up @@ -453,6 +455,14 @@ export default class TrixEditorElement extends HTMLElement {
this.editor?.loadHTML(this.defaultValue)
}

// Element callbacks

attributeChangedCallback(name, oldValue, newValue) {
if (name === "connected" && this.isConnected && oldValue != null && oldValue !== newValue) {
requestAnimationFrame(() => this.reconnect())
}
}

// Controller delegate methods

notify(message, data) {
Expand Down Expand Up @@ -485,13 +495,27 @@ export default class TrixEditorElement extends HTMLElement {
}
this.editorController.registerSelectionManager()
this.#delegate.connectedCallback()

this.toggleAttribute("connected", true)
autofocus(this)
}
}

disconnectedCallback() {
this.editorController?.unregisterSelectionManager()
this.#delegate.disconnectedCallback()
this.toggleAttribute("connected", false)
}

reconnect() {
this.removeInternalToolbar()
this.disconnectedCallback()
this.connectedCallback()
Comment on lines +512 to +513
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this approach work with an existing <trix-toolbar> element? What about a <trix-editor> element with a [toolbar] attribute that references another element elsewhere in the page? Are the toolbars re-initialized as expected without attaching a duplicate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points, I'll see how to make these cases work. You are right that it will lose bootstrapped toolbars. I'll make sure it works well with both scenarios (existing toolbar and bootstrapped).

Copy link
Member Author

Choose a reason for hiding this comment

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

I added some logic to (1) flag internal (bootstrapped) trix toolbars and (2) remove those when reconnecting. This works well in my test.s

}

removeInternalToolbar() {
this.internalToolbar?.remove()
this.internalToolbar = null
}

// Form support
Expand Down