From 226043e6aba1cbdcde4e82d93b732a54e10fc1cb Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 12:10:01 +0200 Subject: [PATCH 1/7] Ensure that user provided SCSS has precedent over quarto generated scss also for dark theme Follow up on b7a5449 which fixed that for the light theme fixes #10817 --- news/changelog-1.6.md | 1 + src/format/html/format-html-scss.ts | 44 +++++++++++++---------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/news/changelog-1.6.md b/news/changelog-1.6.md index efcc5accd6f..0036a4e9325 100644 --- a/news/changelog-1.6.md +++ b/news/changelog-1.6.md @@ -16,6 +16,7 @@ All changes included in 1.6: - Fix `kbd` element styling on dark themes. - ([#10761](https://github.com/quarto-dev/quarto-cli/issues/10761)): Add support for `licence: CC0` to automatically link to Creative Commons licence [CC0 1.0](https://creativecommons.org/publicdomain/zero/1.0/). +- ([#10817](https://github.com/quarto-dev/quarto-cli/issues/10817)): Ensure that user provided SCSS has precedent over quarto generated scss also for dark theme. ## `revealjs` Format diff --git a/src/format/html/format-html-scss.ts b/src/format/html/format-html-scss.ts index ea88ec5dfbc..9e7867608f5 100644 --- a/src/format/html/format-html-scss.ts +++ b/src/format/html/format-html-scss.ts @@ -317,9 +317,9 @@ export function resolveTextHighlightingLayer( if (themeDescriptor) { const readTextColor = (name: string) => { const textStyles = themeDescriptor.json["text-styles"]; - if (textStyles && typeof (textStyles) === "object") { + if (textStyles && typeof textStyles === "object") { const commentColor = (textStyles as Record)[name]; - if (commentColor && typeof (commentColor) === "object") { + if (commentColor && typeof commentColor === "object") { const textColor = (commentColor as Record)["text-color"]; return textColor; @@ -368,19 +368,19 @@ function resolveThemeLayer( let theme = undefined; let defaultDark = false; - if (typeof (themes) === "string") { + if (typeof themes === "string") { // The themes is just a string theme = { light: [themes] }; } else if (Array.isArray(themes)) { // The themes is an array theme = { light: themes }; - } else if (typeof (themes) === "object") { + } else if (typeof themes === "object") { // The themes are an object - look at each key and // deal with them either as a string or a string[] const themeArr = (theme?: unknown): string[] => { const themes: string[] = []; if (theme) { - if (typeof (theme) === "string") { + if (typeof theme === "string") { themes.push(theme); } else if (Array.isArray(theme)) { themes.push(...theme); @@ -417,7 +417,7 @@ function resolveThemeLayer( ? layerTheme(input, theme.dark, quartoThemesDir) : undefined; if (darkLayerContext) { - darkLayerContext.layers.push(...sassLayers); + darkLayerContext.layers.unshift(...sassLayers); const darkHighlightingLayer = resolveTextHighlightingLayer( input, format, @@ -522,7 +522,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { const colorDefaults: string[] = []; const navbar = (metadata[kWebsite] as Metadata)?.[kSiteNavbar]; - if (navbar && typeof (navbar) === "object") { + if (navbar && typeof navbar === "object") { // Forward navbar background color const navbarBackground = (navbar as Record)[kBackground]; if (navbarBackground !== undefined) { @@ -532,9 +532,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "navbar-bg", navbarBackground, - typeof (navbarBackground) === "string" - ? asBootstrapColor - : undefined, + typeof navbarBackground === "string" ? asBootstrapColor : undefined, ), ), ); @@ -549,9 +547,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "navbar-fg", navbarForeground, - typeof (navbarForeground) === "string" - ? asBootstrapColor - : undefined, + typeof navbarForeground === "string" ? asBootstrapColor : undefined, ), ), ); @@ -575,7 +571,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { const sidebars = (metadata[kWebsite] as Metadata)?.[kSiteSidebar]; const sidebar = Array.isArray(sidebars) ? sidebars[0] - : typeof (sidebars) === "object" + : typeof sidebars === "object" ? (sidebars as Metadata) : undefined; @@ -589,7 +585,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "sidebar-bg", sidebarBackground, - typeof (sidebarBackground) === "string" + typeof sidebarBackground === "string" ? asBootstrapColor : undefined, ), @@ -612,7 +608,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "sidebar-fg", sidebarForeground, - typeof (sidebarForeground) === "string" + typeof sidebarForeground === "string" ? asBootstrapColor : undefined, ), @@ -640,7 +636,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { } const footer = (metadata[kWebsite] as Metadata)?.[kPageFooter] as Metadata; - if (footer !== undefined && typeof (footer) === "object") { + if (footer !== undefined && typeof footer === "object") { // Forward footer color const footerBg = footer[kBackground]; if (footerBg !== undefined) { @@ -650,7 +646,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "footer-bg", footerBg, - typeof (footerBg) === "string" ? asBootstrapColor : undefined, + typeof footerBg === "string" ? asBootstrapColor : undefined, ), ), ); @@ -665,7 +661,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( "footer-fg", footerFg, - typeof (footerFg) === "string" ? asBootstrapColor : undefined, + typeof footerFg === "string" ? asBootstrapColor : undefined, ), ), ); @@ -689,7 +685,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { } // If the footer border is a color, set that - if (footerBorder !== undefined && typeof (footerBorder) === "string") { + if (footerBorder !== undefined && typeof footerBorder === "string") { resolveBootstrapColorDefault(footerBorder, colorDefaults); variables.push( outputVariable( @@ -704,7 +700,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { // Forward any footer color const footerColor = footer[kColor]; - if (footerColor && typeof (footerColor) === "string") { + if (footerColor && typeof footerColor === "string") { resolveBootstrapColorDefault(footerColor, colorDefaults); variables.push( outputVariable( @@ -729,7 +725,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { sassVariable( kCodeBorderLeft, codeblockLeftBorder, - typeof (codeblockLeftBorder) === "string" + typeof codeblockLeftBorder === "string" ? asBootstrapColor : undefined, ), @@ -746,7 +742,7 @@ export const quartoBootstrapDefaults = (metadata: Metadata) => { variables.push(outputVariable(sassVariable( kCodeBlockBackground, codeblockBackground, - typeof (codeblockBackground) === "string" ? asBootstrapColor : undefined, + typeof codeblockBackground === "string" ? asBootstrapColor : undefined, ))); } @@ -775,7 +771,7 @@ function resolveBootstrapColorDefault(value: unknown, variables: string[]) { } function bootstrapColorDefault(value: unknown) { - if (typeof (value) === "string") { + if (typeof value === "string") { return bootstrapColorVars[value]; } } From 8c37079bd9154027940d6c9922b1949023e9ba31 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 14:56:00 +0200 Subject: [PATCH 2/7] Add some playwright test for background color in light / dark mode switch --- .../html/dark-light-theme-custom/.gitignore | 2 ++ .../html/dark-light-theme-custom/custom.scss | 8 +++++++ .../html/dark-light-theme-custom/index.qmd | 15 +++++++++++++ .../playwright/tests/html-themes.spec.ts | 22 +++++++++++++++++++ 4 files changed, 47 insertions(+) create mode 100644 tests/docs/playwright/html/dark-light-theme-custom/.gitignore create mode 100644 tests/docs/playwright/html/dark-light-theme-custom/custom.scss create mode 100644 tests/docs/playwright/html/dark-light-theme-custom/index.qmd create mode 100644 tests/integration/playwright/tests/html-themes.spec.ts diff --git a/tests/docs/playwright/html/dark-light-theme-custom/.gitignore b/tests/docs/playwright/html/dark-light-theme-custom/.gitignore new file mode 100644 index 00000000000..2cca5373e92 --- /dev/null +++ b/tests/docs/playwright/html/dark-light-theme-custom/.gitignore @@ -0,0 +1,2 @@ +*_files/ +*.html \ No newline at end of file diff --git a/tests/docs/playwright/html/dark-light-theme-custom/custom.scss b/tests/docs/playwright/html/dark-light-theme-custom/custom.scss new file mode 100644 index 00000000000..57641a49a2f --- /dev/null +++ b/tests/docs/playwright/html/dark-light-theme-custom/custom.scss @@ -0,0 +1,8 @@ +/*-- scss:defaults --*/ + +/*-- scss:rules --*/ +.quarto-title-banner { + margin-bottom: 1em; + color: #dee2e6; + background: red; +} diff --git a/tests/docs/playwright/html/dark-light-theme-custom/index.qmd b/tests/docs/playwright/html/dark-light-theme-custom/index.qmd new file mode 100644 index 00000000000..47f078f690e --- /dev/null +++ b/tests/docs/playwright/html/dark-light-theme-custom/index.qmd @@ -0,0 +1,15 @@ +--- +title: "Quarto Playground" +title-block-banner: true +format: + html: + theme: + light: + - cosmo + - custom.scss + dark: + - darkly + - custom.scss +--- + +This is a playground for Quarto. diff --git a/tests/integration/playwright/tests/html-themes.spec.ts b/tests/integration/playwright/tests/html-themes.spec.ts new file mode 100644 index 00000000000..68f17f49417 --- /dev/null +++ b/tests/integration/playwright/tests/html-themes.spec.ts @@ -0,0 +1,22 @@ +import { test, expect } from '@playwright/test'; + +test('Dark and light theme respect user themes', async ({ page }) => { + // This document use a custom theme file that change the background color of the title banner + // Same user defined color should be used in both dark and light theme + await page.goto('./html/dark-light-theme-custom/'); + const locatr = await page.locator('div').filter({ hasText: 'Quarto Playground' }).first() + await expect(locatr).toHaveCSS('background-color', 'rgb(255, 0, 0)'); + await page.getByRole('link').nth(1).click(); + const locatr2 = await page.locator('div').filter({ hasText: 'Quarto Playground' }).first() + await expect(locatr2).toHaveCSS('background-color', 'rgb(255, 0, 0)'); +}); + +test('Dark theming toggle change to dark background ', async ({ page }) => { + await page.goto('./html/dark-light-theme-custom/'); + const locatr = page.getByText('Quarto Playground This is a'); + await expect(locatr).toHaveCSS('background-color', 'rgb(255, 255, 255)'); + // switching to dark theme using toggle + await page.locator("a.quarto-color-scheme-toggle").click(); + const locatr2 = await page.locator('div').filter({ hasText: 'Quarto Playground' }).first() + await expect(locatr2).toHaveCSS('background-color', 'rgb(255, 0, 0)'); +}); \ No newline at end of file From db4cd2c868580f64a0603f57fd491294036d0682 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 16:00:49 +0200 Subject: [PATCH 3/7] fix locator --- tests/integration/playwright/tests/html-themes.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/playwright/tests/html-themes.spec.ts b/tests/integration/playwright/tests/html-themes.spec.ts index 68f17f49417..84d53f7b866 100644 --- a/tests/integration/playwright/tests/html-themes.spec.ts +++ b/tests/integration/playwright/tests/html-themes.spec.ts @@ -6,7 +6,7 @@ test('Dark and light theme respect user themes', async ({ page }) => { await page.goto('./html/dark-light-theme-custom/'); const locatr = await page.locator('div').filter({ hasText: 'Quarto Playground' }).first() await expect(locatr).toHaveCSS('background-color', 'rgb(255, 0, 0)'); - await page.getByRole('link').nth(1).click(); + await page.locator("a.quarto-color-scheme-toggle").click(); const locatr2 = await page.locator('div').filter({ hasText: 'Quarto Playground' }).first() await expect(locatr2).toHaveCSS('background-color', 'rgb(255, 0, 0)'); }); From 1258bbd07e5a23e2011e465444e9d3bb32e7e6b7 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 17:10:26 +0200 Subject: [PATCH 4/7] Upload playwright report as artifact in Github workflow --- .github/workflows/test-smokes.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index cbd765df20b..f23ae4dc7f2 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -283,8 +283,17 @@ jobs: echo "Pull Request URL - ${{ steps.cpr.outputs.pull-request-url }}" echo "Pull Request Action Performed - ${{ steps.cpr.outputs.pull-request-operation }}" - - uses: actions/upload-artifact@v3 + - name: Upload test timing file + uses: actions/upload-artifact@v3 if: matrix.time-test && ( failure() || cancelled()) with: name: timed test file path: tests/timing-for-ci.txt + + - uses: actions/upload-artifact@v3 + # PLaywright test only runs on Linux for now + if: ${{ !cancelled() && runner.os != 'Windows' }} + with: + name: playwright-report + path: ./tests/integration/playwright/playwright-report/ + retention-days: 30 From 54ea27e4c04f979a1e85788fac8f475fb1f00e89 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 17:11:02 +0200 Subject: [PATCH 5/7] Run playwright testing really as part of Deno test --- tests/integration/playwright-tests.test.ts | 33 ++++++++++++++-------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/tests/integration/playwright-tests.test.ts b/tests/integration/playwright-tests.test.ts index fdd360136be..781ab9526d7 100644 --- a/tests/integration/playwright-tests.test.ts +++ b/tests/integration/playwright-tests.test.ts @@ -14,6 +14,7 @@ import { import { cleanoutput } from "../smoke/render/render.ts"; import { execProcess } from "../../src/core/process.ts"; import { quartoDevCmd } from "../utils.ts"; +import { assert } from "testing/asserts.ts"; async function fullInit() { await initYamlIntelligenceResourcesFromFilesystem(); @@ -29,7 +30,7 @@ setInitializer(fullInit); await initState(); // const promises = []; -const fileNames = []; +const fileNames: string[] = []; for (const { path: fileName } of globOutput) { const input = fileName; @@ -43,15 +44,25 @@ for (const { path: fileName } of globOutput) { fileNames.push(fileName); } +Deno.test("Playwright tests are passing", async () => { + try { + // run playwright + const res = await execProcess({ + cmd: [Deno.build.os == "windows" ? "npx.cmd" : "npx", "playwright", "test"], + cwd: "integration/playwright", + }); -try { - // run playwright - await execProcess({ - cmd: [Deno.build.os == "windows" ? "npx.cmd" : "npx", "playwright", "test"], - cwd: "integration/playwright", - }); -} finally { - for (const fileName of fileNames) { - cleanoutput(fileName, "html"); + if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) { + const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`; + console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`); + } + assert( + res.success, + "Failed tests with playwright. Look at playwright report for more details." + ); + }finally { + for (const fileName of fileNames) { + cleanoutput(fileName, "html"); + } } -} +}); From 02b42ec464ef042f96abf9fec960276fe318a23f Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 17:31:35 +0200 Subject: [PATCH 6/7] Correctly print the message in github action when playwright tests fails --- tests/integration/playwright-tests.test.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/integration/playwright-tests.test.ts b/tests/integration/playwright-tests.test.ts index 781ab9526d7..11686f319ca 100644 --- a/tests/integration/playwright-tests.test.ts +++ b/tests/integration/playwright-tests.test.ts @@ -14,7 +14,7 @@ import { import { cleanoutput } from "../smoke/render/render.ts"; import { execProcess } from "../../src/core/process.ts"; import { quartoDevCmd } from "../utils.ts"; -import { assert } from "testing/asserts.ts"; +import { fail } from "testing/asserts.ts"; async function fullInit() { await initYamlIntelligenceResourcesFromFilesystem(); @@ -51,16 +51,15 @@ Deno.test("Playwright tests are passing", async () => { cmd: [Deno.build.os == "windows" ? "npx.cmd" : "npx", "playwright", "test"], cwd: "integration/playwright", }); - - if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) { - const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`; - console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`); + if (!res.success) { + if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) { + const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`; + console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`); + } + fail("Failed tests with playwright. Look at playwright report for more details.") } - assert( - res.success, - "Failed tests with playwright. Look at playwright report for more details." - ); - }finally { + + } finally { for (const fileName of fileNames) { cleanoutput(fileName, "html"); } From c3de54b18ef571532dfe6a9c8e85054ede8339b5 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 17 Sep 2024 18:45:01 +0200 Subject: [PATCH 7/7] Explicitly ignore test on Windows --- tests/integration/playwright-tests.test.ts | 39 ++++++++++++---------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/integration/playwright-tests.test.ts b/tests/integration/playwright-tests.test.ts index 11686f319ca..f2354c63e1a 100644 --- a/tests/integration/playwright-tests.test.ts +++ b/tests/integration/playwright-tests.test.ts @@ -44,24 +44,29 @@ for (const { path: fileName } of globOutput) { fileNames.push(fileName); } -Deno.test("Playwright tests are passing", async () => { - try { - // run playwright - const res = await execProcess({ - cmd: [Deno.build.os == "windows" ? "npx.cmd" : "npx", "playwright", "test"], - cwd: "integration/playwright", - }); - if (!res.success) { - if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) { - const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`; - console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`); +Deno.test({ + name: "Playwright tests are passing", + // currently we run playwright tests only on Linux + ignore: Deno.build.os === "windows", + fn: async () => { + try { + // run playwright + const res = await execProcess({ + cmd: [Deno.build.os == "windows" ? "npx.cmd" : "npx", "playwright", "test"], + cwd: "integration/playwright", + }); + if (!res.success) { + if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) { + const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`; + console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`); + } + fail("Failed tests with playwright. Look at playwright report for more details.") + } + + } finally { + for (const fileName of fileNames) { + cleanoutput(fileName, "html"); } - fail("Failed tests with playwright. Look at playwright report for more details.") - } - - } finally { - for (const fileName of fileNames) { - cleanoutput(fileName, "html"); } } });