diff --git a/blocks/edit/da-title/da-title.js b/blocks/edit/da-title/da-title.js index 9fabeb43..18a66cae 100644 --- a/blocks/edit/da-title/da-title.js +++ b/blocks/edit/da-title/da-title.js @@ -215,6 +215,14 @@ export default class DaTitle extends LitElement { } } + async handleNavigate(e) { + const modified = e.button !== 0 || e.metaKey || e.ctrlKey || e.shiftKey || e.altKey; + if (!this.beforeNavigate || modified) return; + e.preventDefault(); + const { href } = e.currentTarget; + if (await this.beforeNavigate() !== false) window.location.href = href; + } + async handleAction(action) { this._status = null; this._isSending = true; @@ -417,6 +425,7 @@ export default class DaTitle extends LitElement {
${this.details.parentName}

${this.details.name}

diff --git a/blocks/sheet/sheet.js b/blocks/sheet/sheet.js index 49bae136..61a0252a 100644 --- a/blocks/sheet/sheet.js +++ b/blocks/sheet/sheet.js @@ -2,8 +2,13 @@ import { LitElement, html, nothing } from 'da-lit'; import getPathDetails from '../shared/pathDetails.js'; import { getNx } from '../../scripts/utils.js'; import '../edit/da-title/da-title.js'; -import { getData } from './utils/index.js'; -import { staleCheck, showDaDialog, restoreVersion } from './utils/utils.js'; +import { commitActiveEditors, getData } from './utils/index.js'; +import { + flushPendingSave, + staleCheck, + showDaDialog, + restoreVersion, +} from './utils/utils.js'; import { convertSheets } from '../edit/utils/helpers.js'; const { default: getStyle } = await import(`${getNx()}/utils/styles.js`); @@ -176,6 +181,11 @@ export default async function init(el) { daSheet.className = 'da-sheet'; wrapper.append(daSheet); + daTitle.beforeNavigate = () => { + commitActiveEditors(daSheet.jexcel); + return flushPendingSave().catch(() => false); + }; + // Sheet & Version Wrapper const versionWrapper = document.createElement('div'); versionWrapper.classList.add('da-version-wrapper'); diff --git a/blocks/sheet/utils/index.js b/blocks/sheet/utils/index.js index d39f3da3..a99907a1 100644 --- a/blocks/sheet/utils/index.js +++ b/blocks/sheet/utils/index.js @@ -10,6 +10,12 @@ const SHEET_TEMPLATE = { minDimensions: [20, 20], sheetName: 'data' }; let permissions; let canWrite; +export function commitActiveEditors(sheets) { + sheets?.forEach((sheet) => { + if (sheet.edition) sheet.closeEditor(sheet.edition[0], true); + }); +} + function resetSheets(el) { document.querySelector('da-sheet-tabs')?.remove(); if (!el.jexcel) return; diff --git a/blocks/sheet/utils/utils.js b/blocks/sheet/utils/utils.js index f9f2bc89..03ca461a 100644 --- a/blocks/sheet/utils/utils.js +++ b/blocks/sheet/utils/utils.js @@ -1,4 +1,4 @@ -import { convertSheets, debounce, saveToDa } from '../../edit/utils/helpers.js'; +import { convertSheets, saveToDa } from '../../edit/utils/helpers.js'; import { getNx2Api } from '../../../scripts/utils.js'; const DEBOUNCE_TIME = 1000; @@ -77,7 +77,7 @@ class StaleCheck { export const staleCheck = new StaleCheck(); -export const saveSheets = async (sheets) => { +export const saveSheets = async (sheets, pathname = window.location.hash.replace('#', '')) => { const convertedJson = convertSheets(sheets); document.querySelector('da-sheet-panes').data = convertedJson; @@ -85,8 +85,6 @@ export const saveSheets = async (sheets) => { // last-write-wins between concurrent editors. Drift triggers the onStale flow. if (await staleCheck.checkForDrift()) return false; - const { hash } = window.location; - const pathname = hash.replace('#', ''); const dasSave = await saveToDa(pathname, sheets); if (!dasSave.ok) { // eslint-disable-next-line no-console @@ -97,7 +95,24 @@ export const saveSheets = async (sheets) => { return true; }; -const debouncedSaveSheets = debounce(saveSheets, DEBOUNCE_TIME); +let pendingSave; +let saveTimeout; +let savePromise = Promise.resolve(true); + +export function flushPendingSave() { + clearTimeout(saveTimeout); + saveTimeout = undefined; + + if (pendingSave) { + const { sheets, pathname } = pendingSave; + pendingSave = undefined; + savePromise = savePromise + .catch(() => false) + .then(() => saveSheets(sheets, pathname)); + } + + return savePromise; +} export async function restoreVersion(daTitle, daSheet, versionData) { const initSheet = (await import('./index.js')).default; @@ -110,7 +125,12 @@ export function handleSave(jexcel, view) { staleCheck.markEdited(); if (view === 'config') return; if (staleCheck.isBlocked) return; - debouncedSaveSheets(jexcel); + clearTimeout(saveTimeout); + pendingSave = { + sheets: jexcel, + pathname: window.location.hash.replace('#', ''), + }; + saveTimeout = setTimeout(flushPendingSave, DEBOUNCE_TIME); } export async function showDaDialog({ title, body, confirmLabel }) { diff --git a/test/e2e/tests/sheet.spec.js b/test/e2e/tests/sheet.spec.js index 212efbf9..67c016e4 100644 --- a/test/e2e/tests/sheet.spec.js +++ b/test/e2e/tests/sheet.spec.js @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test'; import { getTestSheetURL, waitForSave } from '../utils/page.js'; -test('New sheet', async ({ page }, workerInfo) => { +test('Last cell edit persists when immediately navigating to the parent folder', async ({ page }, workerInfo) => { test.setTimeout(30000); const url = getTestSheetURL('sheet1', workerInfo); @@ -22,7 +22,17 @@ test('New sheet', async ({ page }, workerInfo) => { await page.locator('[data-x="0"][data-y="1"]').dblclick(); await page.locator('td input').fill(enteredText); - await page.waitForTimeout(3000); + // Leave while the cell editor is still active and before the autosave debounce expires. + // The parent link commits the open editor and flushes the save, then navigates to the + // parent folder. Waiting for that navigation guarantees the save landed (beforeNavigate + // awaits it) and avoids racing it with the reload below. + await page.locator('.da-title-name-label').click(); + await page.waitForURL((u) => !String(u).includes('/sheet#')); + + await page.goto(url); + await expect(page.locator('da-sheet-tabs')).toBeVisible(); + await expect(page.locator('[data-x="0"][data-y="1"]')).toHaveText(enteredText); + await page.close(); }); diff --git a/test/unit/blocks/edit/da-title/da-title.test.js b/test/unit/blocks/edit/da-title/da-title.test.js index 7f799abd..7a41b14b 100644 --- a/test/unit/blocks/edit/da-title/da-title.test.js +++ b/test/unit/blocks/edit/da-title/da-title.test.js @@ -89,6 +89,25 @@ describe('DaTitle', () => { expect(h1.textContent).to.equal('page'); }); + it('waits for beforeNavigate before following the parent link', async () => { + el = await fixture(); + let release; + let called = false; + el.beforeNavigate = () => new Promise((resolve) => { + called = true; + release = resolve; + }); + + const link = el.shadowRoot.querySelector('.da-title-name-label'); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + link.dispatchEvent(event); + + expect(event.defaultPrevented).to.be.true; + expect(called).to.be.true; + release(false); + await nextFrame(); + }); + it('renders actions wrapper', async () => { el = await fixture(); const actionsWrapper = el.shadowRoot.querySelector('.da-title-actions'); diff --git a/test/unit/blocks/sheet/index.test.js b/test/unit/blocks/sheet/index.test.js index 5b3db8dd..89ffa439 100644 --- a/test/unit/blocks/sheet/index.test.js +++ b/test/unit/blocks/sheet/index.test.js @@ -21,6 +21,29 @@ function mockJspreadsheetTabs(container, data) { container.jexcel = data.map((d) => ({ name: d.sheetName, options: {} })); } +describe('commitActiveEditors', () => { + it('commits only worksheets with an active editor', () => { + // jspreadsheet sets `edition` to [cell, innerHTML, colIndex, rowIndex] while a + // cell editor is open; closeEditor expects the cell element (edition[0]). + const activeCell = document.createElement('td'); + const calls = []; + const sheets = [ + { + edition: [activeCell, 'Key', 0, 1], + closeEditor: (...args) => calls.push(args), + }, + { + edition: null, + closeEditor: (...args) => calls.push(args), + }, + ]; + + sh.commitActiveEditors(sheets); + + expect(calls).to.deep.equal([[activeCell, true]]); + }); +}); + describe('init - double restore', () => { let el; let daTitle; diff --git a/test/unit/blocks/sheet/utils-utils.test.js b/test/unit/blocks/sheet/utils-utils.test.js index 0f39b59d..49781e2e 100644 --- a/test/unit/blocks/sheet/utils-utils.test.js +++ b/test/unit/blocks/sheet/utils-utils.test.js @@ -3,7 +3,12 @@ import { expect } from '@esm-bundle/chai'; const { setNx } = await import('../../../../scripts/utils.js'); setNx('/test/fixtures/nx', { hostname: 'example.com' }); -const { saveSheets, handleSave, staleCheck } = await import('../../../../blocks/sheet/utils/utils.js'); +const { + saveSheets, + handleSave, + flushPendingSave, + staleCheck, +} = await import('../../../../blocks/sheet/utils/utils.js'); // The new api.js makes an hlx6 upgrade probe before each source op. The probe // must respond without an x-api-upgrade-available header so the source URL @@ -133,5 +138,21 @@ describe('sheet/utils utils', () => { await new Promise((r) => { setTimeout(r, 1200); }); expect(captured).to.contain('/source/o/r/test'); }); + + it('Flushes a pending save immediately to its original path', async () => { + window.location.hash = '#/o/r/original'; + let captured; + window.fetch = wrap((url) => { + if (typeof url === 'string' && url.includes('/source/')) captured = url; + return Promise.resolve(new Response('', { status: 200 })); + }); + + handleSave([buildSheet('a', [['x']])], 'edit'); + window.location.hash = '#/o/r/other'; + const result = await flushPendingSave(); + + expect(result).to.be.true; + expect(captured).to.contain('/source/o/r/original'); + }); }); });