feat(web): cleanup sync ui

Author: abraham

packages/translations/source/en.xlf

@@ -5,9 +5,6 @@
 <trans-unit id="dialogs.feedback.thanks">
   <source>Thanks for your feedback.</source>
 </trans-unit>
-<trans-unit id="dialogs.feedback.unsynced">
-  <source>Not sent yet. It sends when you're online.</source>
-</trans-unit>
 <trans-unit id="dialogs.feedback.content-rating">
   <source>Content quality:</source>
 </trans-unit>
@@ -518,9 +515,6 @@
 <trans-unit id="shell.sync.offline-many">
   <source>Offline · <x id="0" equiv-text="${pending}"/> changes to sync</source>
 </trans-unit>
-<trans-unit id="shell.sync.unsynced">
-  <source>Saved on this device. Syncs when you're online.</source>
-</trans-unit>
 <trans-unit id="store.offline.needs-network">
   <source>Connect to the internet to send this.</source>
 </trans-unit>

packages/web/src/components/dialogs/feedback-block.test.ts

@@ -92,24 +92,6 @@ describe('feedback-block', () => {
       'aria-hidden',
       'true',
     );
-    expect(shadowRoot.querySelector('.thanks .unsynced')).toBeNull();
-  });
-
-  it('says when saved feedback has not been sent yet', async () => {
-    const { element, shadowRoot } = await fixture<FeedbackBlock>(
-      html`<feedback-block></feedback-block>`,
-    );
-    element.sessionId = 'session-id';
-    setStoreState({
-      user: new Initialized(),
-      feedback: { data: new Success([feedback]), subscription: new Initialized() },
-      sync: { online: false, pending: { feedback: ['session-id'] } },
-    } as unknown as Partial<RootState>);
-    await element.updateComplete;
-
-    expect(shadowRoot.querySelector('.thanks .unsynced')).toHaveTextContent(
-      "Not sent yet. It sends when you're online.",
-    );
   });
 
   it('hides the delete button when there is no saved feedback', async () => {

packages/web/src/components/dialogs/feedback-block.ts

@@ -14,7 +14,6 @@ import {
   setFeedback,
 } from '../../store/feedback';
 import { queueComplexSnackbar, queueSnackbar } from '../../store/snackbars';
-import { selectPending } from '../../store/sync';
 import type { UserState } from '../../store/user';
 import '../shared/star-rating';
 import { type StarRatingChangeDetail } from '../shared/star-rating';
@@ -49,12 +48,6 @@ export class FeedbackBlock extends ThemedElement {
         font-weight: 600;
       }
 
-      .thanks .unsynced {
-        flex-basis: 100%;
-        color: var(--hb-color-on-surface-variant);
-        font-weight: 400;
-      }
-
       #commentInput {
         width: 100%;
       }
@@ -94,8 +87,6 @@ export class FeedbackBlock extends ThemedElement {
   private accessor user!: UserState;
   @fromStore((state, host: FeedbackBlock) => selectFeedbackById(state, host.sessionId))
   private accessor feedback!: RemoteData<Error, Feedback | false>;
-  @fromStore((state) => selectPending(state, 'feedback'))
-  private accessor unsyncedFeedback!: string[];
 
   override willUpdate(changedProperties: PropertyValues) {
     if (changedProperties.has('feedback')) {
@@ -111,15 +102,6 @@ export class FeedbackBlock extends ThemedElement {
             ? html`<div class="thanks">
                 ${illustration(thankYou)}
                 <p>${msg('Thanks for your feedback.', { id: 'dialogs.feedback.thanks' })}</p>
-                ${
-                  this.sessionId && this.unsyncedFeedback.includes(this.sessionId)
-                    ? html`<p class="unsynced">
-                        ${msg("Not sent yet. It sends when you're online.", {
-                          id: 'dialogs.feedback.unsynced',
-                        })}
-                      </p>`
-                    : nothing
-                }
               </div>`
             : nothing
         }

packages/web/src/components/schedule/session-element.test.ts

@@ -114,21 +114,6 @@ describe('session-element', () => {
     expect(bookmark).toHaveAttribute('label', 'Bookmark Example Session');
     expect(bookmark.pressed).toBe(true);
     expect(bookmark.querySelector('hoverboard-icon')).toHaveAttribute('name', 'bookmark-check');
-    expect(shadowRoot.querySelector('.unsynced')).toBeNull();
-  });
-
-  it('marks a bookmark that has not synced yet', async () => {
-    const { shadowRoot } = await render({
-      featuredSessions: new Success({ 'session-1': true }),
-      unsyncedBookmarks: ['session-1'],
-    } as never);
-    const bookmark = shadowRoot.querySelector('hb-icon-button')!;
-
-    expect(bookmark).toHaveAttribute(
-      'label',
-      "Bookmark Example Session. Saved on this device. Syncs when you're online.",
-    );
-    expect(bookmark.querySelector('.unsynced')).toHaveAttribute('name', 'cloud-upload');
   });
 
   it('asks to sign in before bookmarking', async () => {

packages/web/src/components/schedule/session-element.ts

@@ -13,7 +13,6 @@ import {
   setUserFeaturedSessions,
 } from '../../store/featured-sessions';
 import { queueComplexSnackbar } from '../../store/snackbars';
-import { selectPending, unsyncedMessage } from '../../store/sync';
 import type { UserState } from '../../store/user';
 import { acceptingFeedback } from '../../utils/feedback';
 import { confetti } from '../../utils/confetti';
@@ -157,16 +156,6 @@ export class SessionElement extends ThemedElement {
       inset-inline-end: var(--hb-space-1);
     }
 
-    .unsynced {
-      position: absolute;
-      inset-block-end: 4px;
-      inset-inline-end: 4px;
-      inline-size: 14px;
-      block-size: 14px;
-      border-radius: 50%;
-      background-color: var(--hb-panel-background);
-    }
-
     @media (prefers-reduced-motion: reduce) {
       .session:hover {
         translate: none;
@@ -187,8 +176,6 @@ export class SessionElement extends ThemedElement {
   accessor session: BuiltSession | undefined;
   @fromStore((state) => selectFeaturedSessionsState(state))
   accessor featuredSessions!: FeaturedSessionsState;
-  @fromStore((state) => selectPending(state, 'featuredSessions'))
-  private accessor unsyncedBookmarks!: string[];
 
   // Depends on the time, so it is only set in the browser.
   @state()
@@ -270,23 +257,16 @@ export class SessionElement extends ThemedElement {
     }
     if (!__HB_FEATURES__.mySchedule) return nothing;
     const bookmarked = this.isBookmarked;
-    const unsynced = bookmarked && this.unsyncedBookmarks.includes(session.id);
-    const label = msg(str`Bookmark ${session.title}`, { id: 'schedule.session.bookmark' });
     return html`
       <hb-icon-button
         class="action bookmark"
-        label="${unsynced ? `${label}. ${unsyncedMessage()}` : label}"
+        label="${msg(str`Bookmark ${session.title}`, { id: 'schedule.session.bookmark' })}"
         .pressed="${bookmarked}"
         @click="${this.toggleBookmark}"
       >
         <hoverboard-icon
           name="${bookmarked ? 'bookmark-check' : 'bookmark-plus'}"
         ></hoverboard-icon>
-        ${
-          unsynced
-            ? html`<hoverboard-icon class="unsynced" name="cloud-upload"></hoverboard-icon>`
-            : nothing
-        }
       </hb-icon-button>
     `;
   }

packages/web/src/components/shared/hoverboard-icon.test.ts

@@ -58,6 +58,8 @@ describe('hoverboard-icon', () => {
     'density-small',
     'density-medium',
     'density-large',
+    'cloud-off',
+    'cloud-upload',
   ])('renders the %s icon', async (name) => {
     const { shadowRoot } = await fixture(html`<hoverboard-icon name="${name}"></hoverboard-icon>`);
 
@@ -92,6 +94,15 @@ describe('hoverboard-icon', () => {
     expect(missing).toEqual([]);
   });
 
+  it('bundles the icons that show offline, which cannot load a chunk then', async () => {
+    const source = (await import('./hoverboard-icon.ts?raw')).default;
+
+    for (const name of ['cloud-off', 'cloud-upload']) {
+      expect(source).toMatch(new RegExp(`^import \\w+ from './icons/${name}';$`, 'm'));
+      expect(source).not.toContain(`import('./icons/${name}')`);
+    }
+  });
+
   it('renders nothing for an unknown icon name', async () => {
     const element = document.createElement('hoverboard-icon') as HoverboardIcon;
     element.name = 'not-real';

packages/web/src/components/shared/hoverboard-icon.ts

@@ -1,6 +1,8 @@
 import { css, html, isServer, type PropertyValues, type SVGTemplateResult } from 'lit';
 import { customElement, property, state } from 'lit/decorators.js';
 import { ThemedElement } from '../themed-element';
+import cloudOff from './icons/cloud-off';
+import cloudUpload from './icons/cloud-upload';
 
 // Lit replacement for the retired `<iron-icon icon="hoverboard:name">` Polymer iconset.
 // Each icon is its own module under ./icons so only the icons a page uses are loaded.
@@ -48,8 +50,6 @@ const ICONS: Record<string, () => Promise<{ default: SVGTemplateResult }>> = {
   people: () => import('./icons/people'),
   registration: () => import('./icons/registration'),
   'bell-off': () => import('./icons/bell-off'),
-  'cloud-off': () => import('./icons/cloud-off'),
-  'cloud-upload': () => import('./icons/cloud-upload'),
   'bell-outline': () => import('./icons/bell-outline'),
   monitor: () => import('./icons/monitor'),
   sun: () => import('./icons/sun'),
@@ -59,7 +59,11 @@ const ICONS: Record<string, () => Promise<{ default: SVGTemplateResult }>> = {
   'density-large': () => import('./icons/density-large'),
 };
 
-const loaded = new Map<string, SVGTemplateResult>();
+// The icons that show offline are bundled, since their chunks might not be cached.
+const loaded = new Map<string, SVGTemplateResult>([
+  ['cloud-off', cloudOff],
+  ['cloud-upload', cloudUpload],
+]);
 
 @customElement('hoverboard-icon')
 export class HoverboardIcon extends ThemedElement {
@@ -104,12 +108,12 @@ export class HoverboardIcon extends ThemedElement {
   }
 
   private loadIcon(name: string) {
+    const cached = loaded.get(name);
     const load = ICONS[name];
-    if (!load) {
+    if (!cached && !load) {
       return;
     }
-    const cached = loaded.get(name);
-    const loading = (cached ? Promise.resolve({ default: cached }) : load())
+    const loading = (cached ? Promise.resolve({ default: cached }) : load!())
       .then((module) => {
         loaded.set(name, module.default);
         if (this.name === name) {

packages/web/src/components/shell/sync-status.test.ts

@@ -14,12 +14,28 @@ describe('sync-status', () => {
     expect(element.shadowRoot?.querySelector('details')).toBeNull();
   });
 
-  it('says when the site is offline, with the changes to sync and what works offline', async () => {
+  it('shows only an icon while offline with nothing to sync', async () => {
+    setStoreState({ sync: { online: false, pending: {} } });
+    const { shadowRoot, shadowRootForWithin } = await fixture(html`<sync-status></sync-status>`);
+    const status = within(shadowRootForWithin).getByRole('status');
+
+    expect(status).toHaveTextContent(/^Offline$/);
+    expect(status.querySelector('[aria-hidden]')).toBeNull();
+    expect(shadowRoot.querySelector('hoverboard-icon')).toHaveAttribute('name', 'cloud-off');
+  });
+
+  it('shows the number of changes to sync, with what works offline', async () => {
     setStoreState({ sync: { online: false, pending: { featuredSessions: ['a', 'b'] } } });
-    const { shadowRootForWithin } = await fixture(html`<sync-status></sync-status>`);
+    const { shadowRoot, shadowRootForWithin } = await fixture(html`<sync-status></sync-status>`);
     const view = within(shadowRootForWithin);
+    const status = view.getByRole('status');
 
-    expect(view.getByRole('status')).toHaveTextContent('Offline · 2 changes to sync');
+    expect(status.querySelector('[aria-hidden="true"]')).toHaveTextContent(/^2$/);
+    expect(shadowRoot.querySelector('hoverboard-icon')).toHaveAttribute('name', 'cloud-upload');
+    expect(status.querySelector('.visually-hidden')).toHaveTextContent(
+      'Offline · 2 changes to sync',
+    );
+    expect(status).toHaveAttribute('title', 'Offline · 2 changes to sync');
     expect(
       view.getByText(/saved on this device, and sync when you are online/),
     ).toBeInTheDocument();
@@ -28,7 +44,9 @@ describe('sync-status', () => {
   it('says when it syncs changes made offline', async () => {
     setStoreState({ sync: { online: true, pending: { feedback: ['a'] } } });
     const { shadowRootForWithin } = await fixture(html`<sync-status></sync-status>`);
+    const status = within(shadowRootForWithin).getByRole('status');
 
-    expect(within(shadowRootForWithin).getByRole('status')).toHaveTextContent('Syncing…');
+    expect(status.querySelector('[aria-hidden="true"]')).toHaveTextContent(/^1$/);
+    expect(status.querySelector('.visually-hidden')).toHaveTextContent('Syncing…');
   });
 });

packages/web/src/components/shell/sync-status.ts

@@ -21,13 +21,17 @@ export class SyncStatus extends ThemedElement {
     summary {
       display: inline-flex;
       align-items: center;
+      justify-content: center;
       gap: var(--hb-space-1);
-      min-block-size: 32px;
-      padding: var(--hb-space-1) var(--hb-space-3);
+      min-inline-size: 40px;
+      block-size: 40px;
+      box-sizing: border-box;
+      padding: 0 var(--hb-space-2);
       border-radius: var(--hb-radius-full);
       background-color: var(--hb-color-accent-2-container);
       color: var(--hb-color-on-accent-2-container);
-      font: 500 var(--hb-text-sm) / 1.2 var(--hb-font-mono);
+      font: 600 var(--hb-text-sm) / 1 var(--hb-font-mono);
+      white-space: nowrap;
       list-style: none;
       cursor: pointer;
     }
@@ -42,8 +46,18 @@ export class SyncStatus extends ThemedElement {
     }
 
     hoverboard-icon {
-      inline-size: 18px;
-      block-size: 18px;
+      inline-size: 20px;
+      block-size: 20px;
+    }
+
+    /* Still read by screen readers. */
+    .visually-hidden {
+      position: absolute;
+      inline-size: 1px;
+      block-size: 1px;
+      overflow: hidden;
+      clip-path: inset(50%);
+      white-space: nowrap;
     }
 
     .help {
@@ -79,9 +93,10 @@ export class SyncStatus extends ThemedElement {
     if (!label) return nothing;
     return html`
       <details>
-        <summary role="status">
-          <hoverboard-icon name="${this.online ? 'cloud-upload' : 'cloud-off'}"></hoverboard-icon>
-          ${label}
+        <summary role="status" title="${label}">
+          ${this.pending ? html`<span aria-hidden="true">${this.pending}</span>` : nothing}
+          <hoverboard-icon name="${this.pending ? 'cloud-upload' : 'cloud-off'}"></hoverboard-icon>
+          <span class="visually-hidden">${label}</span>
         </summary>
         <p class="help">
           ${msg(

packages/web/src/store/sync/index.test.ts

@@ -3,7 +3,6 @@ import reducer, {
   canWriteNow,
   resetPending,
   selectOnline,
-  selectPending,
   selectPendingCount,
   setPendingIds,
   syncLabel,
@@ -44,7 +43,6 @@ describe('sync', () => {
     setPendingIds('featuredSessions', ['session-1', 'session-2']);
     setPendingIds('feedback', ['session-3']);
     expect(selectPendingCount(store.getState())).toBe(3);
-    expect(selectPending(store.getState(), 'feedback')).toEqual(['session-3']);
 
     go(true);
     expect(lastSnackbar()).not.toBe('Back online. Your changes are saved.');