NTP: Remove search icon and input suffix from omnibar, tweak surface colours (#1871)

Author: noisysocks

special-pages/pages/new-tab/app/omnibar/components/Omnibar.module.css

@@ -58,7 +58,7 @@
 
 .root:has([role="listbox"]) .popup,
 .root:not(:has([role="listbox"])) .field {
-    backdrop-filter: blur(48px);
+    backdrop-filter: blur(16px);
     background: var(--ntp-surface-tertiary);
     border-radius: var(--border-radius-lg);
 }

special-pages/pages/new-tab/app/omnibar/components/OmnibarConsumer.js

@@ -41,7 +41,7 @@ function OmnibarReadyState({ config: { enableAi = true, showAiSetting = true, mo
     return (
         <>
             {showAiSetting && <AiSetting enableAi={enableAi} setEnableAi={setEnableAi} />}
-            <Omnibar mode={mode} setMode={setMode} enableAi={enableAi} />
+            <Omnibar mode={mode} setMode={setMode} enableAi={showAiSetting && enableAi} />
         </>
     );
 }

special-pages/pages/new-tab/app/omnibar/components/SearchForm.js

@@ -1,7 +1,6 @@
 import { Fragment, h } from 'preact';
 import { useEffect, useMemo } from 'preact/hooks';
 import { eventToTarget } from '../../../../../shared/handlers.js';
-import { GlobeIcon, SearchIcon } from '../../components/Icons.js';
 import { usePlatformName } from '../../settings.provider.js';
 import { useTypedTranslationWith } from '../../types';
 import { getInputSuffix, getSuggestionCompletionString, startsWithIgnoreCase } from '../utils.js';
@@ -125,7 +124,6 @@ export function SearchForm({ autoFocus, onOpenSuggestion, onSubmit }) {
                 });
             }}
         >
-            {inputSuffix?.kind === 'visit' ? <GlobeIcon inert /> : <SearchIcon inert />}
             <input
                 ref={inputRef}
                 type="text"

special-pages/pages/new-tab/app/omnibar/components/SearchForm.module.css

@@ -1,12 +1,8 @@
 .form {
     align-items: center;
     display: flex;
-    padding: var(--sp-1) 7px;
-
-    svg {
-        flex: none;
-        margin: var(--sp-2);
-    }
+    height: var(--sp-10);
+    padding: var(--sp-1) 7px var(--sp-1) calc(7px + var(--sp-2));
 }
 
 .input {
@@ -17,7 +13,7 @@
     height: var(--sp-8);
     left: 7px;
     padding-bottom: 0;
-    padding-left: calc(var(--sp-2) + var(--sp-4) + var(--sp-2));
+    padding-left: var(--sp-2);
     padding-right: var(--suffix-text-width);
     padding-top: 0;
     position: absolute;

special-pages/pages/new-tab/app/omnibar/unit-tests/utils.spec.js

@@ -6,7 +6,6 @@ import {
     getSuggestionCompletionString,
     getSuggestionSuffix,
     parseURL,
-    isURLish,
     formatURL,
     formatURLForTerm,
     getDuckDuckGoSearchQuery,
@@ -22,23 +21,8 @@ test.describe('getInputSuffix', () => {
         equal(getInputSuffix('', null), null);
     });
 
-    test('returns "Visit $url" for url-like input', () => {
-        let suffix = getInputSuffix('example.com', null);
-        equal(suffix?.kind, 'visit');
-        equal(suffix?.url, 'example.com');
-
-        suffix = getInputSuffix('https://foobar.com/path', null);
-        equal(suffix?.kind, 'visit');
-        equal(suffix?.url, 'foobar.com/path');
-
-        suffix = getInputSuffix('www.foo.com/bar/?q=yes#frag', null);
-        equal(suffix?.kind, 'visit');
-        equal(suffix?.url, 'www.foo.com/bar');
-    });
-
-    test('returns "Search DuckDuckGo" for non-url terms', () => {
-        const suffix = getInputSuffix('pizza delivery near me', null);
-        equal(suffix?.kind, 'searchDuckDuckGo');
+    test('returns null when there is no selected suggestion', () => {
+        equal(getInputSuffix('pizza', null), null);
     });
 
     test('returns "Search DuckDuckGo" if selected suggestion is a phrase', () => {
@@ -361,50 +345,6 @@ test.describe('parseURL', () => {
     });
 });
 
-test.describe('isURLish', () => {
-    test('returns true for simple domain with dot', () => {
-        equal(isURLish('example.com'), true);
-        equal(isURLish('foo.bar'), true);
-        equal(isURLish('some.site'), true);
-        equal(isURLish('my-site.org'), true);
-    });
-
-    test('returns true for full url', () => {
-        equal(isURLish('https://example.com'), true);
-        equal(isURLish('http://www.example.com'), true);
-        equal(isURLish('ftp://files.example.com'), true);
-    });
-
-    test('returns true for domain with path/params/hash', () => {
-        equal(isURLish('example.com/path'), true);
-        equal(isURLish('example.com/index.html'), true);
-        equal(isURLish('example.com?q=search'), true);
-        equal(isURLish('example.com/path#hash'), true);
-        equal(isURLish('example.com:8080'), true);
-    });
-
-    test('returns false for regular strings', () => {
-        equal(isURLish('localhost'), false);
-        equal(isURLish('duck'), false);
-        equal(isURLish('what time is it'), false);
-    });
-
-    test('returns false for empty string', () => {
-        equal(isURLish(''), false);
-    });
-
-    test('returns true for IP addresses', () => {
-        equal(isURLish('127.0.0.1'), true);
-        equal(isURLish('192.168.1.1'), true);
-        equal(isURLish('8.8.8.8:53'), true);
-    });
-
-    test('returns true for URLs with subdomains', () => {
-        equal(isURLish('www.example.com'), true);
-        equal(isURLish('a.b.c.domain.co.uk'), true);
-    });
-});
-
 test.describe('formatURL', () => {
     test('returns full URL by default (scheme, www, trailing slash, search, hash)', () => {
         const url = new URL('https://www.example.com/path/?q=123#myhash');

special-pages/pages/new-tab/app/omnibar/utils.js

@@ -25,43 +25,24 @@
  * @returns {Suffix}
  */
 export function getInputSuffix(term, selectedSuggestion) {
-    if (!term) {
-        return null;
-    }
-
-    if (selectedSuggestion) {
-        return getSuggestionInputSuffix(selectedSuggestion, term);
-    }
+    // Intentionally differs from the macOS app's implementation, which returns
+    // "Search DuckDuckGo" or "Visit $url" when there is a term and no selection
+    if (!term || !selectedSuggestion) return null;
 
-    if (isURLish(term)) {
-        const url = parseURL(term);
-        if (!url) throw new Error('isURLish returned true but parseURL failed');
-        return { kind: 'visit', url: formatURL(url, { scheme: false, trailingSlash: false, search: false, hash: false }) };
-    } else {
-        return { kind: 'searchDuckDuckGo' };
-    }
-}
-
-/**
- * @param {Suggestion} suggestion
- * @param {string} term
- * @returns {Suffix}
- */
-function getSuggestionInputSuffix(suggestion, term) {
-    switch (suggestion.kind) {
+    switch (selectedSuggestion.kind) {
         case 'phrase':
             return { kind: 'searchDuckDuckGo' };
         case 'website': {
-            const url = parseURL(suggestion.url);
+            const url = parseURL(selectedSuggestion.url);
             if (!url) return null;
             return { kind: 'visit', url: formatURL(url, { scheme: false, trailingSlash: false, search: false, hash: false }) };
         }
         case 'bookmark':
         case 'historyEntry':
         case 'internalPage': {
-            const title = getSuggestionTitle(suggestion, term);
-            const autocompletion = getSuggestionCompletionString(suggestion, term);
-            const url = parseURL(suggestion.url);
+            const title = getSuggestionTitle(selectedSuggestion, term);
+            const autocompletion = getSuggestionCompletionString(selectedSuggestion, term);
+            const url = parseURL(selectedSuggestion.url);
             if (title && title !== autocompletion) {
                 return { kind: 'raw', text: title };
             } else if (url) {
@@ -174,15 +155,6 @@ export function parseURL(string) {
     return null;
 }
 
-/**
- * @param {string} string
- * @returns {boolean}
- */
-export function isURLish(string) {
-    // @todo: This is overly simplistic.
-    return string.includes('.') && parseURL(string) !== null;
-}
-
 /**
  * @param {URL} url
  * @param {object} [options]

special-pages/pages/new-tab/app/styles/ntp-theme.css

@@ -60,10 +60,10 @@ body {
 
     /* Global Colors & Styles - https://www.figma.com/design/3W4vi0zX8hrpQc7zInQQB6/%F0%9F%8E%A8-Global-Colors---Styles?node-id=11-1&p=f&vars=1&m=dev */
     /* @todo: This palette is what the native apps use, we should gradually move over to it */
-    --ntp-surface-tertiary: #FFFFFF;
-    --ntp-text-primary: #1F1F1F;
-    --ntp-text-tertiary: rgba(31, 31, 31, 0.4);
-    --ntp-icons-primary: rgba(31, 31, 31, 0.84);
+    --ntp-surface-tertiary: rgba(255, 255, 255, 0.88); /* This differs from Figma. We're adding alpha so that surfaces blend with custom backgrounds. */
+    --ntp-text-primary: rgba(0, 0, 0, 0.96);
+    --ntp-text-tertiary: rgba(0, 0, 0, 0.42);
+    --ntp-icons-primary: rgba(0, 0, 0, 0.84);
     --ntp-icons-tertiary: rgba(0, 0, 0, 0.36);
     --ntp-accent-primary: var(--color-blue-50);
     --ntp-accent-secondary: var(--color-blue-60);
@@ -94,9 +94,9 @@ body {
 
     /* Global Colors & Styles - https://www.figma.com/design/3W4vi0zX8hrpQc7zInQQB6/%F0%9F%8E%A8-Global-Colors---Styles?node-id=11-1&p=f&vars=1&m=dev */
     /* @todo: This palette is what the native apps use, we should gradually move over to it */
-    --ntp-surface-tertiary: rgba(71, 71, 71, 0.66); /* @fixme: This differs from Figma. We're adding alpha so that surfaces blend with custom backgrounds. */
-    --ntp-text-primary: rgba(255, 255, 255, 0.9);
-    --ntp-text-tertiary: rgba(255, 255, 255, 0.4);
+    --ntp-surface-tertiary: rgba(0, 0, 0, 0.72); /* This differs from Figma. We're adding alpha so that surfaces blend with custom backgrounds. */
+    --ntp-text-primary: rgba(255, 255, 255, 0.96);
+    --ntp-text-tertiary: rgba(255, 255, 255, 0.36);
     --ntp-icons-primary: rgba(255, 255, 255, 0.78);
     --ntp-icons-tertiary: rgba(255, 255, 255, 0.24);
     --ntp-accent-primary: var(--color-blue-20);

special-pages/pages/new-tab/integration-tests/new-tab.screenshots.spec.js

@@ -167,13 +167,36 @@ test.describe('NTP screenshots', { tag: ['@screenshots'] }, () => {
             });
         });
 
-        test('ai disabled', async ({ page }, workerInfo) => {
-            const ntp = NewtabPage.create(page, workerInfo);
-            const omnibar = new OmnibarPage(ntp);
-            await ntp.reducedMotion();
-            await ntp.openPage({ additional: { omnibar: 'true', 'omnibar.enableAi': 'false' } });
-            await omnibar.ready();
-            await expect(page).toHaveScreenshot('omnibar-ai-disabled.png', { maxDiffPixels });
+        test.describe('sidebar', () => {
+            test('ai enabled', async ({ page }, workerInfo) => {
+                const ntp = NewtabPage.create(page, workerInfo);
+                const omnibar = new OmnibarPage(ntp);
+                await ntp.reducedMotion();
+                await ntp.openPage({ additional: { omnibar: 'true' } });
+                await omnibar.ready();
+                await omnibar.customizeButton().click();
+                await expect(page).toHaveScreenshot('omnibar-sidebar-ai-enabled.png', { maxDiffPixels });
+            });
+
+            test('ai disabled', async ({ page }, workerInfo) => {
+                const ntp = NewtabPage.create(page, workerInfo);
+                const omnibar = new OmnibarPage(ntp);
+                await ntp.reducedMotion();
+                await ntp.openPage({ additional: { omnibar: 'true', 'omnibar.enableAi': 'false' } });
+                await omnibar.ready();
+                await omnibar.customizeButton().click();
+                await expect(page).toHaveScreenshot('omnibar-sidebar-ai-disabled.png', { maxDiffPixels });
+            });
+
+            test('hide ai setting', async ({ page }, workerInfo) => {
+                const ntp = NewtabPage.create(page, workerInfo);
+                const omnibar = new OmnibarPage(ntp);
+                await ntp.reducedMotion();
+                await ntp.openPage({ additional: { omnibar: 'true', 'omnibar.showAiSetting': 'false' } });
+                await omnibar.ready();
+                await omnibar.customizeButton().click();
+                await expect(page).toHaveScreenshot('omnibar-sidebar-hide-ai-setting.png', { maxDiffPixels });
+            });
         });
     });
 });