ntp: prevent gradients bleeding through (#1382)

shakyShane · web-flow · commit fb7fff484f53 · 2025-01-09T11:35:16.000Z
* ntp: prevent gradients bleeding through

* fixed tests

* set explicit widths on icon+text to prevent overflows
diff --git a/special-pages/pages/new-tab/app/components/App.module.css b/special-pages/pages/new-tab/app/components/App.module.css
@@ -20,6 +20,9 @@ body {
 body:has([data-reset-layout="true"]) .tube {
     padding-top: 0;
 }
+body[data-animate-background="true"] {
+    transition: background-color .3s;
+}
 
 :global(.layout-centered) {
     margin-inline: auto;
diff --git a/special-pages/pages/new-tab/app/components/BackgroundProvider.js b/special-pages/pages/new-tab/app/components/BackgroundProvider.js
@@ -5,6 +5,7 @@ import { values } from '../customizer/values.js';
 import { useContext, useState } from 'preact/hooks';
 import { CustomizerContext } from '../customizer/CustomizerProvider.js';
 import { detectThemeFromHex } from '../customizer/utils.js';
+import { useSignalEffect } from '@preact/signals';
 
 /**
  * @import { BackgroundVariant, BrowserTheme } from "../../types/new-tab"
@@ -59,40 +60,56 @@ export function BackgroundConsumer({ browser }) {
     const { data } = useContext(CustomizerContext);
     const background = data.value.background;
 
-    switch (background.kind) {
-        case 'default': {
-            return <div className={styles.root} data-testid="BackgroundConsumer" data-background-kind="default" data-theme={browser} />;
+    useSignalEffect(() => {
+        const background = data.value.background;
+
+        // reflect some values onto the <body> tag
+        document.body.dataset.backgroundKind = background.kind;
+        let nextBodyBackground = '';
+
+        if (background.kind === 'gradient') {
+            const gradient = values.gradients[background.value];
+            nextBodyBackground = gradient.fallback;
+        }
+        if (background.kind === 'color') {
+            const color = values.colors[background.value];
+            nextBodyBackground = color.hex;
+        }
+        if (background.kind === 'hex') {
+            nextBodyBackground = background.value;
         }
+        if (background.kind === 'userImage') {
+            const isDark = background.value.colorScheme === 'dark';
+            nextBodyBackground = isDark ? 'var(--default-dark-bg)' : 'var(--default-light-bg)';
+        }
+        if (background.kind === 'default') {
+            nextBodyBackground = browser.value === 'dark' ? 'var(--default-dark-bg)' : 'var(--default-light-bg)';
+        }
+
+        document.body.style.setProperty('background-color', nextBodyBackground);
+
+        // let animations occur, after properties above have been flushed to the DOM
+        if (!document.body.dataset.animateBackground) {
+            requestAnimationFrame(() => {
+                document.body.dataset.animateBackground = 'true';
+            });
+        }
+    });
+
+    switch (background.kind) {
+        case 'color':
+        case 'default':
         case 'hex': {
-            return (
-                <div
-                    class={styles.root}
-                    data-animate="true"
-                    data-testid="BackgroundConsumer"
-                    style={{
-                        backgroundColor: background.value,
-                    }}
-                ></div>
-            );
+            return null;
         }
-        case 'color': {
-            const color = values.colors[background.value];
-            return (
-                <div
-                    class={styles.root}
-                    data-animate="true"
-                    data-background-color={color.hex}
-                    data-testid="BackgroundConsumer"
-                    style={{
-                        backgroundColor: color.hex,
-                    }}
-                ></div>
-            );
+        case 'userImage': {
+            const img = background.value;
+            return <ImageCrossFade src={img.src} />;
         }
         case 'gradient': {
             const gradient = values.gradients[background.value];
             return (
-                <Fragment key="gradient">
+                <Fragment>
                     <ImageCrossFade src={gradient.path}></ImageCrossFade>
                     <div
                         className={styles.root}
@@ -102,17 +119,13 @@ export function BackgroundConsumer({ browser }) {
                             opacity: 0.5,
                             mixBlendMode: 'soft-light',
                         }}
-                    ></div>
+                    />
                 </Fragment>
             );
         }
-        case 'userImage': {
-            const img = background.value;
-            return <ImageCrossFade src={img.src} />;
-        }
         default: {
             console.warn('Unreachable!');
-            return <div className={styles.root}></div>;
+            return null;
         }
     }
 }
diff --git a/special-pages/pages/new-tab/app/components/BackgroundReceiver.module.css b/special-pages/pages/new-tab/app/components/BackgroundReceiver.module.css
@@ -8,13 +8,7 @@
     pointer-events: none;
 
     &[data-animate="true"] {
-        transition: background .25s ease-in-out;
-    }
-    &[data-background-kind="default"][data-theme=dark] {
-        background: var(--default-dark-bg);
-    }
-    &[data-background-kind="default"][data-theme=light] {
-        background: var(--default-light-bg);
+        transition: background .3s;
     }
 }
 
diff --git a/special-pages/pages/new-tab/app/customizer/integration-tests/customizer.page.js b/special-pages/pages/new-tab/app/customizer/integration-tests/customizer.page.js
@@ -129,21 +129,20 @@ export class CustomizerPage {
 
     async hasDefaultBackground() {
         const { page } = this.ntp;
-        await expect(page.getByTestId('BackgroundConsumer')).toHaveCSS('background-color', 'rgb(250, 250, 250)');
+        await expect(page.locator('body')).toHaveCSS('background-color', 'rgb(250, 250, 250)');
     }
 
     async hasDefaultDarkBackground() {
         const { page } = this.ntp;
-        await expect(page.getByTestId('BackgroundConsumer')).toHaveCSS('background-color', 'rgb(51, 51, 51)');
+        await expect(page.locator('body')).toHaveCSS('background-color', 'rgb(51, 51, 51)');
     }
 
     /**
-     * @param {keyof typeof values.colors} color
+     * @param {string} expectedRGB
      */
-    async hasColorBackground(color) {
+    async hasColorBackground(expectedRGB) {
         const { page } = this.ntp;
-        const value = values.colors[color];
-        await expect(page.getByTestId('BackgroundConsumer')).toHaveAttribute('data-background-color', value.hex);
+        await expect(page.locator('body')).toHaveCSS('background-color', expectedRGB);
     }
 
     async selectsColor() {
diff --git a/special-pages/pages/new-tab/app/customizer/integration-tests/customizer.spec.js b/special-pages/pages/new-tab/app/customizer/integration-tests/customizer.spec.js
@@ -80,7 +80,7 @@ test.describe('newtab customizer', () => {
             kind: 'color',
             value: 'color01',
         });
-        await cp.hasColorBackground('color01');
+        await cp.hasColorBackground('rgb(0, 0, 0)');
     });
     test('loads with the default background and accepts theme update', async ({ page }, workerInfo) => {
         const ntp = NewtabPage.create(page, workerInfo);
diff --git a/special-pages/pages/new-tab/app/favorites/components/Tile.module.css b/special-pages/pages/new-tab/app/favorites/components/Tile.module.css
@@ -1,4 +1,6 @@
 .item {
+    --icon-width: 4rem;
+
     display: block;
     position: relative;
     text-decoration: none;
@@ -29,8 +31,8 @@
     display: grid;
     align-content: center;
     justify-items: center;
-    width: 4rem;
-    height: 4rem;
+    width: var(--icon-width);
+    height: var(--icon-width);
     margin-bottom: 4px;
     border-radius: var(--border-radius-lg);
 }
@@ -89,6 +91,7 @@
 }
 
 .text {
+    width: var(--icon-width);
     text-align: center;
     font-size: calc(10 * var(--px-in-rem));
     line-height: 1.1;

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,9 @@ body {`
`20`	`20`	`body:has([data-reset-layout="true"]) .tube {`
`21`	`21`	`padding-top: 0;`
`22`	`22`	`}`
	`23`	`+body[data-animate-background="true"] {`
	`24`	`+ transition: background-color .3s;`
	`25`	`+}`
`23`	`26`
`24`	`27`	`:global(.layout-centered) {`
`25`	`28`	`margin-inline: auto;`
Original file line number	Diff line number	Diff line change
`@@ -8,13 +8,7 @@`
`8`	`8`	`pointer-events: none;`
`9`	`9`
`10`	`10`	`&[data-animate="true"] {`
`11`		`- transition: background .25s ease-in-out;`
`12`		`- }`
`13`		`- &[data-background-kind="default"][data-theme=dark] {`
`14`		`- background: var(--default-dark-bg);`
`15`		`- }`
`16`		`- &[data-background-kind="default"][data-theme=light] {`
`17`		`- background: var(--default-light-bg);`
	`11`	`+ transition: background .3s;`
`18`	`12`	`}`
`19`	`13`	`}`
`20`	`14`