Skip to content

Commit

Permalink
Keep Trix dynamic styles in the head (#1133)
Browse files Browse the repository at this point in the history
* Keep Trix dynamic styles in the head

Trix dynamically adds some styles to the head that we need to keep around.
Otherwise, the editor will not render correctly after a page change.

We can detect those styles because Trix also adds a `data-tag-name`
attribute to the style elements it adds.

Ideally, we would move those styles to Trix's CSS file, but for now we'll
just skip removing them. We don't want to force everyone upgrading to
Turbo v8 to also upgrade Trix.

Ref:

- https://github.com/basecamp/trix/blob/06d8b1db5fb682d007c5ca041884f6297674c8b7/src/trix/elements/trix_editor_element.js#L108-L162
- https://github.com/basecamp/trix/blob/06d8b1db5fb682d007c5ca041884f6297674c8b7/src/trix/core/helpers/custom_elements.js#L11

* Use data-turbo-permanent to keep styles around

And use it to keep the progress bar styles around, instead of relying
on an id.
  • Loading branch information
Alberto Fernández-Capel committed Jan 22, 2024
1 parent a73f6f1 commit 32b8c30
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/core/drive/page_renderer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { activateScriptElement, waitForLoad } from "../../util"
import { Renderer } from "../renderer"
import { ProgressBarID } from "./progress_bar"

export class PageRenderer extends Renderer {
static renderElement(currentElement, newElement) {
Expand Down Expand Up @@ -184,7 +183,13 @@ export class PageRenderer extends Renderer {
}

get unusedHeadStylesheetElements() {
return this.oldHeadStylesheetElements.filter((element) => element.id !== ProgressBarID)
return this.oldHeadStylesheetElements.filter((element) => {
return !(element.hasAttribute("data-turbo-permanent") ||
// Trix dynamically adds styles to the head that we want to keep around which have a
// `data-page-name` attribute. Long term we should moves those styles to Trix's CSS file
// but for now we'll just skip removing them
element.hasAttribute("data-page-name"))
})
}

get oldHeadStylesheetElements() {
Expand Down
1 change: 1 addition & 0 deletions src/core/drive/progress_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export class ProgressBar {
createStylesheetElement() {
const element = document.createElement("style")
element.id = ProgressBarID
element.setAttribute("data-turbo-permanent", "")
element.type = "text/css"
element.textContent = ProgressBar.defaultCSS
if (this.cspNonce) {
Expand Down
3 changes: 3 additions & 0 deletions src/tests/functional/drive_stylesheet_merging_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ test.beforeEach(async ({ page }) => {
})

test("navigating removes unused style elements", async ({ page }) => {
assert.ok(await hasSelector(page, 'style[id="turbo-progress-bar"]'))

await page.locator("#go-right").click()
await nextBody(page)

assert.ok(await hasSelector(page, 'style[id="turbo-progress-bar"]'))
assert.ok(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/common.css"]'))
assert.ok(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/right.css"]'))
assert.notOk(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/left.css"]'))
Expand Down

0 comments on commit 32b8c30

Please sign in to comment.