From 6743b57b88689818de615edcaadfa5f9915e930a Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:20:19 -0600 Subject: [PATCH 1/6] feat: add backup settings to app storage This is draft from copilot workspace as a starter. Add a setting option to toggle backup of settings to app storage. * Add a new checkbox for "Backup settings to app storage" in `src/settings.ts`. * Update `src/utils.ts` to handle the backup toggle and sync the choice when enabled. * Modify `src/sw.ts` to include the backup settings during installation or update. * Add tests in `test/unit/settings.test.ts` to verify the backup toggle functionality and safety check for exceeding local storage. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/maxmilton/new-tab?shareId=XXXX-XXXX-XXXX-XXXX). --- src/settings.ts | 25 +++++++++++++++++++++++++ src/sw.ts | 7 +++++++ src/utils.ts | 34 ++++++++++++++++++++++++++++++++++ test/unit/settings.test.ts | 31 +++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+) diff --git a/src/settings.ts b/src/settings.ts index 8a9dbd115..3426cd529 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -107,6 +107,7 @@ interface Refs { se: HTMLUListElement; sd: HTMLUListElement; reset: HTMLButtonElement; + backup: HTMLInputElement; } const meta = compile(` @@ -134,6 +135,12 @@ const meta = compile(` </label> </div> + <div class=row> + <label> + <input @backup type=checkbox class=box> Backup settings to app storage + </label> + </div> + <div class=row> <label>Sections</label> <fieldset> @@ -244,6 +251,23 @@ const Settings = () => { } }; + refs.backup.onchange = async () => { + if (refs.backup.checked) { + const bytesInUse = await chrome.storage.local.getBytesInUse(); + const quotaBytes = chrome.storage.local.QUOTA_BYTES; + if (bytesInUse > quotaBytes) { + refs.feedback.textContent = `Warning: Settings exceed local storage limit. Current size: ${bytesInUse} bytes, Allowed size: ${quotaBytes} bytes, Total size over: ${bytesInUse - quotaBytes} bytes.`; + refs.backup.checked = false; + } else { + void chrome.storage.local.set({ + backup: true, + }); + } + } else { + void chrome.storage.local.remove('backup'); + } + }; + // eslint-disable-next-line no-multi-assign refs.se.ondragover = refs.sd.ondragover = (event) => { event.preventDefault(); @@ -267,6 +291,7 @@ const Settings = () => { void updateTheme(themeName); refs.b.checked = !storage.b; + refs.backup.checked = !!storage.backup; updateOrder([orderEnabled, orderDisabled], true); return root; diff --git a/src/sw.ts b/src/sw.ts index 1504971f9..bfb53a01b 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -23,6 +23,13 @@ chrome.runtime.onInstalled.addListener(async () => { t: themes[settings.tn ?? 'auto'], }); + // Include backup setting during installation or update + if (settings.backup) { + void chrome.storage.local.set({ + backup: settings.backup, + }); + } + // TODO: Open settings page on install? // if (details.reason === ('install' as chrome.runtime.OnInstalledReason.INSTALL)) { // void chrome.tabs.create({ diff --git a/src/utils.ts b/src/utils.ts index 3869d9f2b..121ce4739 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -51,3 +51,37 @@ export const handleClick = (event: MouseEvent): false | void => { s.focus(); }; + +// Add a new property `backup` to the `storage` object to store the backup setting +export const storage: UserStorageData = await chrome.storage.local.get(); +storage.backup = storage.backup ?? false; + +// Update the `handleClick` function to include logic for syncing settings when backup is enabled +export const handleClick = (event: MouseEvent): false | void => { + let node = event.target as + | (Node & { __click?(event: MouseEvent): false | undefined }) + | null; + const url = (node as Node & { href?: string }).href; + + while (node) { + if (node.__click) { + return node.__click(event); + } + node = node.parentNode; + } + + if (url && url[0] !== 'h') { + if (event.ctrlKey) { + void chrome.tabs.create({ url }); + } else { + void chrome.tabs.update({ url }); + } + return false; + } + + s.focus(); + + if (storage.backup) { + void chrome.storage.local.set(storage); + } +}; diff --git a/test/unit/settings.test.ts b/test/unit/settings.test.ts index 78e0d5906..1972b5523 100644 --- a/test/unit/settings.test.ts +++ b/test/unit/settings.test.ts @@ -61,6 +61,37 @@ test('gets stored user settings once on load', async () => { expect(spy).toHaveBeenCalledTimes(1); }); +test('toggles backup setting in chrome.storage.local', async () => { + expect.assertions(4); + await load(); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"][name="backup"]'); + expect(backupCheckbox).toBeTruthy(); + if (backupCheckbox) { + backupCheckbox.checked = true; + backupCheckbox.dispatchEvent(new Event('change')); + expect(chrome.storage.local.set).toHaveBeenCalledWith({ backup: true }); + + backupCheckbox.checked = false; + backupCheckbox.dispatchEvent(new Event('change')); + expect(chrome.storage.local.remove).toHaveBeenCalledWith('backup'); + } +}); + +test('displays warning and unchecks backup box if settings exceed local storage', async () => { + expect.assertions(3); + const spy = spyOn(chrome.storage.local, 'getBytesInUse').mockResolvedValue(6000000); // Mock value exceeding quota + await load(); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"][name="backup"]'); + const feedbackDiv = document.querySelector('div[feedback]'); + expect(backupCheckbox).toBeTruthy(); + if (backupCheckbox) { + backupCheckbox.checked = true; + backupCheckbox.dispatchEvent(new Event('change')); + expect(backupCheckbox.checked).toBe(false); + expect(feedbackDiv?.textContent).toContain('Warning: Settings exceed local storage limit.'); + } +}); + const css = await Bun.file('dist/settings.css').text(); describe('CSS', () => { From a3bec9353234e9cedda8d3d7d63603e52dbea145 Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:57:30 -0600 Subject: [PATCH 2/6] Add backup settings to app storage * **src/settings.ts** - Add a new checkbox for "Backup settings to app storage" in the settings form - Update the `refs` interface to include the new checkbox - Add an `onchange` event handler to the new checkbox to update the backup setting in `chrome.storage.local` - Add a safety check to display a warning if settings exceed local storage and uncheck the box * **src/utils.ts** - Add a new property `backup` to the `storage` object to store the backup setting - Update the `handleClick` function to include logic for syncing settings when backup is enabled * **src/sw.ts** - Update the `onInstalled` listener to include the backup setting during installation or update * **test/unit/settings.test.ts** - Add tests to verify the backup toggle functionality in the settings page - Add tests to verify the safety check for exceeding local storage - Remove unused variable `spy` --- src/utils.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/utils.ts b/src/utils.ts index 121ce4739..e1955e450 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -50,6 +50,10 @@ export const handleClick = (event: MouseEvent): false | void => { } s.focus(); + + if (storage.backup) { + void chrome.storage.local.set(storage); + } }; // Add a new property `backup` to the `storage` object to store the backup setting From 6e0d397b9c66d6958f114bf939f60867adefad2c Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 19:11:35 -0600 Subject: [PATCH 3/6] --- src/utils.ts | 43 +------------------------------------- test/unit/settings.test.ts | 10 ++++----- 2 files changed, 6 insertions(+), 47 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index e1955e450..6113fc9e4 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -2,6 +2,7 @@ import type { UserStorageData } from './types'; performance.mark('Load Storage'); export const storage: UserStorageData = await chrome.storage.local.get(); +storage.backup = storage.backup ?? false; // NOTE: When updating also update references that lookup items by index export const DEFAULT_SECTION_ORDER = [ @@ -19,48 +20,6 @@ declare const s: HTMLInputElement; // stage1, plus special handling for browser internal links (e.g. chrome://) // https://github.com/maxmilton/stage1/blob/master/src/events.ts // eslint-disable-next-line @typescript-eslint/no-invalid-void-type, consistent-return -export const handleClick = (event: MouseEvent): false | void => { - let node = event.target as - | (Node & { __click?(event: MouseEvent): false | undefined }) - | null; - // const link = node as HTMLAnchorElement; - // const url = link.href; - const url = (node as Node & { href?: string }).href; - - while (node) { - if (node.__click) { - return node.__click(event); - } - node = node.parentNode; - } - - // Only apply special handling to non-http links - if (url && url[0] !== 'h') { - // if (link.target === '_blank' || event.ctrlKey) { - if (event.ctrlKey) { - // Open the location in a new tab - void chrome.tabs.create({ url }); - } else { - // Update the location in the current tab - void chrome.tabs.update({ url }); - } - - // Prevent default behaviour; shorter than `event.preventDefault()` - return false; - } - - s.focus(); - - if (storage.backup) { - void chrome.storage.local.set(storage); - } -}; - -// Add a new property `backup` to the `storage` object to store the backup setting -export const storage: UserStorageData = await chrome.storage.local.get(); -storage.backup = storage.backup ?? false; - -// Update the `handleClick` function to include logic for syncing settings when backup is enabled export const handleClick = (event: MouseEvent): false | void => { let node = event.target as | (Node & { __click?(event: MouseEvent): false | undefined }) diff --git a/test/unit/settings.test.ts b/test/unit/settings.test.ts index 1972b5523..688001042 100644 --- a/test/unit/settings.test.ts +++ b/test/unit/settings.test.ts @@ -114,20 +114,20 @@ describe('CSS', () => { test('does not contain any comments', () => { expect.assertions(4); - expect(css).not.toInclude('/*'); + expect(css).not toInclude('/*'); expect(css).not.toInclude('*/'); - expect(css).not.toInclude('//'); // inline comments or URL protocol - expect(css).not.toInclude('<!'); + expect(css).not toInclude('//'); // inline comments or URL protocol + expect(css).not toInclude('<!'); }); test('does not contain ":root"', () => { expect.assertions(1); - expect(css).not.toInclude(':root'); + expect(css).not toInclude(':root'); }); test('compiled AST is not empty', () => { expect.assertions(1); - expect(ast).not.toBeEmpty(); + expect(ast).not toBeEmpty(); }); test('does not have any rules with a ":root" selector', () => { From 774e69399a2382d10e78eb8abeae7d71909bfe0b Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 19:21:18 -0600 Subject: [PATCH 4/6] --- src/utils.ts | 39 +++++++++++++++++++++++++++++++++++++- test/unit/settings.test.ts | 11 +++++------ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 6113fc9e4..121ce4739 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -2,7 +2,6 @@ import type { UserStorageData } from './types'; performance.mark('Load Storage'); export const storage: UserStorageData = await chrome.storage.local.get(); -storage.backup = storage.backup ?? false; // NOTE: When updating also update references that lookup items by index export const DEFAULT_SECTION_ORDER = [ @@ -20,6 +19,44 @@ declare const s: HTMLInputElement; // stage1, plus special handling for browser internal links (e.g. chrome://) // https://github.com/maxmilton/stage1/blob/master/src/events.ts // eslint-disable-next-line @typescript-eslint/no-invalid-void-type, consistent-return +export const handleClick = (event: MouseEvent): false | void => { + let node = event.target as + | (Node & { __click?(event: MouseEvent): false | undefined }) + | null; + // const link = node as HTMLAnchorElement; + // const url = link.href; + const url = (node as Node & { href?: string }).href; + + while (node) { + if (node.__click) { + return node.__click(event); + } + node = node.parentNode; + } + + // Only apply special handling to non-http links + if (url && url[0] !== 'h') { + // if (link.target === '_blank' || event.ctrlKey) { + if (event.ctrlKey) { + // Open the location in a new tab + void chrome.tabs.create({ url }); + } else { + // Update the location in the current tab + void chrome.tabs.update({ url }); + } + + // Prevent default behaviour; shorter than `event.preventDefault()` + return false; + } + + s.focus(); +}; + +// Add a new property `backup` to the `storage` object to store the backup setting +export const storage: UserStorageData = await chrome.storage.local.get(); +storage.backup = storage.backup ?? false; + +// Update the `handleClick` function to include logic for syncing settings when backup is enabled export const handleClick = (event: MouseEvent): false | void => { let node = event.target as | (Node & { __click?(event: MouseEvent): false | undefined }) diff --git a/test/unit/settings.test.ts b/test/unit/settings.test.ts index 069034bfc..1972b5523 100644 --- a/test/unit/settings.test.ts +++ b/test/unit/settings.test.ts @@ -14,7 +14,6 @@ async function load() { if (input === 'themes.json') { return Promise.resolve(new Response(themes)); } - // eslint-disable-next-line @typescript-eslint/no-base-to-string throw new Error(`Unexpected fetch call: ${String(input)}`); }); // eslint-disable-next-line no-multi-assign @@ -115,20 +114,20 @@ describe('CSS', () => { test('does not contain any comments', () => { expect.assertions(4); - expect(css).not toInclude('/*'); + expect(css).not.toInclude('/*'); expect(css).not.toInclude('*/'); - expect(css).not toInclude('//'); // inline comments or URL protocol - expect(css).not toInclude('<!'); + expect(css).not.toInclude('//'); // inline comments or URL protocol + expect(css).not.toInclude('<!'); }); test('does not contain ":root"', () => { expect.assertions(1); - expect(css).not toInclude(':root'); + expect(css).not.toInclude(':root'); }); test('compiled AST is not empty', () => { expect.assertions(1); - expect(ast).not toBeEmpty(); + expect(ast).not.toBeEmpty(); }); test('does not have any rules with a ":root" selector', () => { From 1b8938ae5c515dd5bad67a829b0b81dd5750b7b3 Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 19:28:13 -0600 Subject: [PATCH 5/6] --- src/utils.ts | 30 ---------------------------- test/unit/settings.test.ts | 40 +++++++++++++++++--------------------- 2 files changed, 18 insertions(+), 52 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 121ce4739..dceac3f21 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -50,36 +50,6 @@ export const handleClick = (event: MouseEvent): false | void => { } s.focus(); -}; - -// Add a new property `backup` to the `storage` object to store the backup setting -export const storage: UserStorageData = await chrome.storage.local.get(); -storage.backup = storage.backup ?? false; - -// Update the `handleClick` function to include logic for syncing settings when backup is enabled -export const handleClick = (event: MouseEvent): false | void => { - let node = event.target as - | (Node & { __click?(event: MouseEvent): false | undefined }) - | null; - const url = (node as Node & { href?: string }).href; - - while (node) { - if (node.__click) { - return node.__click(event); - } - node = node.parentNode; - } - - if (url && url[0] !== 'h') { - if (event.ctrlKey) { - void chrome.tabs.create({ url }); - } else { - void chrome.tabs.update({ url }); - } - return false; - } - - s.focus(); if (storage.backup) { void chrome.storage.local.set(storage); diff --git a/test/unit/settings.test.ts b/test/unit/settings.test.ts index 1972b5523..c45656c62 100644 --- a/test/unit/settings.test.ts +++ b/test/unit/settings.test.ts @@ -61,34 +61,30 @@ test('gets stored user settings once on load', async () => { expect(spy).toHaveBeenCalledTimes(1); }); -test('toggles backup setting in chrome.storage.local', async () => { +test('displays warning and unchecks backup box if settings exceed local storage', async () => { expect.assertions(4); + const spy = spyOn(chrome.storage.local, 'getBytesInUse').mockResolvedValueOnce(11000000); await load(); - const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"][name="backup"]'); - expect(backupCheckbox).toBeTruthy(); - if (backupCheckbox) { - backupCheckbox.checked = true; - backupCheckbox.dispatchEvent(new Event('change')); - expect(chrome.storage.local.set).toHaveBeenCalledWith({ backup: true }); - - backupCheckbox.checked = false; - backupCheckbox.dispatchEvent(new Event('change')); - expect(chrome.storage.local.remove).toHaveBeenCalledWith('backup'); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"]'); + expect(backupCheckbox).toBeInstanceOf(HTMLInputElement); + if (backupCheckbox instanceof HTMLInputElement) { + backupCheckbox.click(); + expect(spy).toHaveBeenCalledTimes(1); + expect(backupCheckbox.checked).toBeFalse(); + const feedback = document.querySelector('div[feedback]'); + expect(feedback?.textContent).toContain('Warning: Settings exceed local storage limit.'); } }); -test('displays warning and unchecks backup box if settings exceed local storage', async () => { - expect.assertions(3); - const spy = spyOn(chrome.storage.local, 'getBytesInUse').mockResolvedValue(6000000); // Mock value exceeding quota +test('syncs backup setting choice when enabled', async () => { + expect.assertions(2); + const spy = spyOn(chrome.storage.local, 'set'); await load(); - const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"][name="backup"]'); - const feedbackDiv = document.querySelector('div[feedback]'); - expect(backupCheckbox).toBeTruthy(); - if (backupCheckbox) { - backupCheckbox.checked = true; - backupCheckbox.dispatchEvent(new Event('change')); - expect(backupCheckbox.checked).toBe(false); - expect(feedbackDiv?.textContent).toContain('Warning: Settings exceed local storage limit.'); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"]'); + expect(backupCheckbox).toBeInstanceOf(HTMLInputElement); + if (backupCheckbox instanceof HTMLInputElement) { + backupCheckbox.click(); + expect(spy).toHaveBeenCalledWith({ backup: true }); } }); From bdb29d429cda856d6d9b8bd7cb969713fb441cb0 Mon Sep 17 00:00:00 2001 From: sheldonhull <sheldonhull@users.noreply.github.com> Date: Fri, 8 Nov 2024 20:33:54 -0600 Subject: [PATCH 6/6] --- src/settings.ts | 9 +++++++-- src/sw.ts | 6 +++--- src/utils.ts | 4 ++-- test/unit/settings.test.ts | 30 ++++++++++++++++++++++++++++-- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/settings.ts b/src/settings.ts index 3426cd529..0afda0a95 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -137,7 +137,7 @@ const meta = compile(` <div class=row> <label> - <input @backup type=checkbox class=box> Backup settings to app storage + <input @backup type=checkbox class=box> Backup and sync settings </label> </div> @@ -262,9 +262,14 @@ const Settings = () => { void chrome.storage.local.set({ backup: true, }); + void chrome.storage.sync.set({ + backup: true, + }); } } else { - void chrome.storage.local.remove('backup'); + void chrome.storage.local.set({ + backup: false, + }); } }; diff --git a/src/sw.ts b/src/sw.ts index bfb53a01b..e39bc52f0 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -23,10 +23,10 @@ chrome.runtime.onInstalled.addListener(async () => { t: themes[settings.tn ?? 'auto'], }); - // Include backup setting during installation or update - if (settings.backup) { + // Include backup and sync setting during installation or update + if (settings.backupSync) { void chrome.storage.local.set({ - backup: settings.backup, + backupSync: settings.backupSync, }); } diff --git a/src/utils.ts b/src/utils.ts index dceac3f21..eb2a79b06 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -51,7 +51,7 @@ export const handleClick = (event: MouseEvent): false | void => { s.focus(); - if (storage.backup) { - void chrome.storage.local.set(storage); + if (storage.backupSync) { + void chrome.storage.sync.set(storage); } }; diff --git a/test/unit/settings.test.ts b/test/unit/settings.test.ts index c45656c62..e6b75ca11 100644 --- a/test/unit/settings.test.ts +++ b/test/unit/settings.test.ts @@ -88,6 +88,32 @@ test('syncs backup setting choice when enabled', async () => { } }); +test('syncs settings to chrome.storage.sync when backup and sync is enabled', async () => { + expect.assertions(2); + const spy = spyOn(chrome.storage.sync, 'set'); + await load(); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"]'); + expect(backupCheckbox).toBeInstanceOf(HTMLInputElement); + if (backupCheckbox instanceof HTMLInputElement) { + backupCheckbox.click(); + expect(spy).toHaveBeenCalledWith({ backup: true }); + } +}); + +test('does not remove backed-up settings when disabling sync', async () => { + expect.assertions(2); + const spy = spyOn(chrome.storage.local, 'set'); + await load(); + const backupCheckbox = document.querySelector('input[type="checkbox"][class="box"]'); + expect(backupCheckbox).toBeInstanceOf(HTMLInputElement); + if (backupCheckbox instanceof HTMLInputElement) { + backupCheckbox.click(); // Enable sync + backupCheckbox.click(); // Disable sync + expect(spy).toHaveBeenCalledWith({ backup: true }); + expect(spy).toHaveBeenCalledWith({ backup: false }); + } +}); + const css = await Bun.file('dist/settings.css').text(); describe('CSS', () => { @@ -118,12 +144,12 @@ describe('CSS', () => { test('does not contain ":root"', () => { expect.assertions(1); - expect(css).not.toInclude(':root'); + expect(css).not toInclude(':root'); }); test('compiled AST is not empty', () => { expect.assertions(1); - expect(ast).not.toBeEmpty(); + expect(ast).not toBeEmpty(); }); test('does not have any rules with a ":root" selector', () => {