From 8fbb573477dbdd8cfcef58304e494b00efabb63b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Thu, 18 Jan 2024 16:19:34 +0000 Subject: [PATCH 1/2] 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 --- src/core/drive/page_renderer.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index d9c78e5ac..700177c64 100644 --- a/src/core/drive/page_renderer.js +++ b/src/core/drive/page_renderer.js @@ -114,7 +114,12 @@ export class PageRenderer extends Renderer { removeUnusedHeadStylesheetElements() { for (const element of this.unusedHeadStylesheetElements) { - document.head.removeChild(element) + // Trix dynamically adds styles to the head that we want to keep around + // Long term we should moves those styles to Trix's CSS file + // but for now we'll just skip removing them + if (!element.dataset.tagName) { + document.head.removeChild(element) + } } } From cfd6edb1bc82cad2d20c6e5bbbfca293da9d5099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Thu, 18 Jan 2024 17:13:15 +0000 Subject: [PATCH 2/2] Use data-turbo-permanent to keep styles around And use it to keep the progress bar styles around, instead of relying on an id. --- src/core/drive/page_renderer.js | 16 ++++++++-------- src/core/drive/progress_bar.js | 1 + .../functional/drive_stylesheet_merging_tests.js | 3 +++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index 700177c64..76072638f 100644 --- a/src/core/drive/page_renderer.js +++ b/src/core/drive/page_renderer.js @@ -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) { @@ -114,12 +113,7 @@ export class PageRenderer extends Renderer { removeUnusedHeadStylesheetElements() { for (const element of this.unusedHeadStylesheetElements) { - // Trix dynamically adds styles to the head that we want to keep around - // Long term we should moves those styles to Trix's CSS file - // but for now we'll just skip removing them - if (!element.dataset.tagName) { - document.head.removeChild(element) - } + document.head.removeChild(element) } } @@ -189,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() { diff --git a/src/core/drive/progress_bar.js b/src/core/drive/progress_bar.js index bd0f6958b..8ce398ca4 100644 --- a/src/core/drive/progress_bar.js +++ b/src/core/drive/progress_bar.js @@ -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) { diff --git a/src/tests/functional/drive_stylesheet_merging_tests.js b/src/tests/functional/drive_stylesheet_merging_tests.js index c51e7421e..6e38c2d82 100644 --- a/src/tests/functional/drive_stylesheet_merging_tests.js +++ b/src/tests/functional/drive_stylesheet_merging_tests.js @@ -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"]'))