From 32b8c3057d82ac004a1ebce1f45463d8493cbe93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Mon, 22 Jan 2024 10:04:40 +0000 Subject: [PATCH] Keep Trix dynamic styles in the head (#1133) * 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. --- src/core/drive/page_renderer.js | 9 +++++++-- src/core/drive/progress_bar.js | 1 + src/tests/functional/drive_stylesheet_merging_tests.js | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index d9c78e5ac..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) { @@ -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() { 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"]'))