diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index 10f3bd4b..b91bb1af 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -395,7 +395,8 @@ BaseView.prototype.getAnchorLinkForSection = function(sectionId) { const fieldIndex = this.cursor.fieldIndex.peek(); const field = fieldIndex !== null ? this.viewSection.viewFields().peek()[fieldIndex] : null; const colRef = field?.colRef.peek(); - return {hash: {sectionId, rowId, colRef}}; + const linkingRowIds = sectionId ? this.gristDoc.getLinkingRowIds(sectionId) : undefined; + return {hash: {sectionId, rowId, colRef, linkingRowIds}}; } // Copy an anchor link for the current row to the clipboard. diff --git a/app/client/components/CursorMonitor.ts b/app/client/components/CursorMonitor.ts index 1c875770..0490c811 100644 --- a/app/client/components/CursorMonitor.ts +++ b/app/client/components/CursorMonitor.ts @@ -59,7 +59,12 @@ export class CursorMonitor extends Disposable { if (!this._restored) { return; } // store position only when we have valid rowId // for some views (like CustomView) cursor position might not reflect actual row - if (pos && pos.rowId !== undefined) { this._storePosition(pos); } + if (pos && pos.rowId !== undefined) { + if (pos.sectionId) { + pos = {...pos, linkingRowIds: doc.getLinkingRowIds(pos.sectionId)}; + } + this._storePosition(pos); + } })); } diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 8a030929..eab011ca 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -72,7 +72,7 @@ import {TableData} from 'app/common/TableData'; import {getGristConfig} from 'app/common/urlUtils'; import {DocStateComparison} from 'app/common/UserAPI'; import {AttachedCustomWidgets, IAttachedCustomWidget, IWidgetType} from 'app/common/widgetTypes'; -import {CursorPos} from 'app/plugin/GristAPI'; +import {CursorPos, UIRowId} from 'app/plugin/GristAPI'; import { bundleChanges, Computed, @@ -1181,6 +1181,29 @@ export class GristDoc extends DisposableWithEvents { return rulesTable.numRecords() > rulesTable.filterRowIds({permissionsText: '', permissions: 63}).length; } + /** + * If the given section is the target of linking, collect and return the active rowIDs up the + * chain of links, returning the list of rowIds starting with the current section's parent. This + * method is intended for when there is ambiguity such as when RefList linking is involved. + * In other cases, returns undefined. + */ + public getLinkingRowIds(sectionId: number): UIRowId[]|undefined { + const linkingRowIds: UIRowId[] = []; + let anyAmbiguity = false; + let section = this.docModel.viewSections.getRowModel(sectionId); + const seen = new Set(); + while (section?.id.peek() && !seen.has(section.id.peek())) { + seen.add(section.id.peek()); + const rowId = section.activeRowId.peek() || 'new'; + if (isRefListType(section.linkTargetCol.peek().type.peek()) || rowId === 'new') { + anyAmbiguity = true; + } + linkingRowIds.push(rowId); + section = section.linkSrcSection.peek(); + } + return anyAmbiguity ? linkingRowIds.slice(1) : undefined; + } + /** * Move to the desired cursor position. If colRef is supplied, the cursor will be * moved to a field with that colRef. Any linked sections that need their cursors @@ -1211,6 +1234,8 @@ export class GristDoc extends DisposableWithEvents { } const srcSection = section.linkSrcSection.peek(); + const linkingRowId = cursorPos.linkingRowIds?.[0]; + const linkingRowIds = cursorPos.linkingRowIds?.slice(1); if (srcSection.id.peek()) { // We're in a linked section, so we need to recurse to make sure the row we want // will be visible. @@ -1218,7 +1243,11 @@ export class GristDoc extends DisposableWithEvents { let controller: any; if (linkTargetCol.colId.peek()) { const destTable = await this._getTableData(section); - controller = destTable.getValue(cursorPos.rowId, linkTargetCol.colId.peek()); + if (cursorPos.rowId === 'new') { + controller = 'new'; + } else { + controller = destTable.getValue(cursorPos.rowId, linkTargetCol.colId.peek()); + } } else { controller = cursorPos.rowId; } @@ -1228,8 +1257,15 @@ export class GristDoc extends DisposableWithEvents { if (!colId && !isSrcSummary) { // Simple case - source linked by rowId, not a summary. if (isList(controller)) { - // Should be a reference list. Pick the first reference. - controller = controller[1]; // [0] is the L type code, [1] is the first value + // Should be a reference list. Use linkingRowId if available and present in the list, + if (linkingRowId && controller.indexOf(linkingRowId) > 0) { + controller = linkingRowId; + } else { + // Otherwise, pick the first reference. + controller = controller[1]; // [0] is the L type code, [1] is the first value + } + } else if (controller === 'new' && linkingRowId) { + controller = linkingRowId; } srcRowId = controller; } else { @@ -1253,12 +1289,13 @@ export class GristDoc extends DisposableWithEvents { } srcRowId = srcTable.getRowIds().find(getFilterFunc(this.docData, query)); } - if (!srcRowId || typeof srcRowId !== 'number') { + if (!srcRowId || (typeof srcRowId !== 'number' && srcRowId !== 'new')) { throw new Error('cannot trace rowId'); } await this.recursiveMoveToCursorPos({ rowId: srcRowId, sectionId: srcSection.id.peek(), + linkingRowIds, }, false, silent, visitedSections.concat([section.id.peek()])); } const view: ViewRec = section.view.peek(); @@ -1694,6 +1731,7 @@ export class GristDoc extends DisposableWithEvents { if (fieldIndex >= 0) { cursorPos.fieldIndex = fieldIndex; } + cursorPos.linkingRowIds = hash.linkingRowIds; } return cursorPos; } diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index eee562dd..486c370c 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -313,7 +313,11 @@ export function encodeUrl(gristConfig: Partial, hashParts.push('a1'); } for (const key of ['sectionId', 'rowId', 'colRef'] as Array) { - const partValue = hash[key]; + let enhancedRowId: string|undefined; + if (key === 'rowId' && hash.linkingRowIds?.length) { + enhancedRowId = [hash.rowId, ...hash.linkingRowIds].join("-"); + } + const partValue = enhancedRowId ?? hash[key]; if (partValue) { const partKey = key === 'rowId' && state.hash?.rickRow ? 'rr' : key[0]; hashParts.push(`${partKey}${partValue}`); @@ -512,7 +516,7 @@ export function decodeUrl(gristConfig: Partial, location: Locat 'sectionId', 'rowId', 'colRef', - ] as Array>; + ] as Array<'sectionId'|'rowId'|'colRef'>; for (const key of keys) { let ch: string; if (key === 'rowId' && hashMap.has('rr')) { @@ -525,6 +529,10 @@ export function decodeUrl(gristConfig: Partial, location: Locat const value = hashMap.get(ch); if (key === 'rowId' && value === 'new') { link[key] = 'new'; + } else if (key === 'rowId' && value && value.includes("-")) { + const rowIdParts = value.split("-").map(p => (p === 'new' ? p : parseInt(p, 10))); + link[key] = rowIdParts[0]; + link.linkingRowIds = rowIdParts.slice(1); } else { link[key] = parseInt(value!, 10); } @@ -1005,6 +1013,7 @@ export interface HashLink { popup?: boolean; rickRow?: boolean; recordCard?: boolean; + linkingRowIds?: UIRowId[]; } // Check whether a urlId is a prefix of the docId, and adequately long to be diff --git a/app/plugin/GristAPI-ti.ts b/app/plugin/GristAPI-ti.ts index 24168dac..c457e42d 100644 --- a/app/plugin/GristAPI-ti.ts +++ b/app/plugin/GristAPI-ti.ts @@ -11,6 +11,7 @@ export const CursorPos = t.iface([], { "rowIndex": t.opt("number"), "fieldIndex": t.opt("number"), "sectionId": t.opt("number"), + "linkingRowIds": t.opt(t.array("UIRowId")), }); export const ComponentKind = t.union(t.lit("safeBrowser"), t.lit("safePython"), t.lit("unsafeNode")); diff --git a/app/plugin/GristAPI.ts b/app/plugin/GristAPI.ts index 056b9ae0..c5c1fa5c 100644 --- a/app/plugin/GristAPI.ts +++ b/app/plugin/GristAPI.ts @@ -65,6 +65,11 @@ export interface CursorPos { * The id of a section that this cursor is in. Ignored when setting a cursor position for a particular view. */ sectionId?: number; + /** + * When in a linked section, CursorPos may include which rows in the controlling sections are + * selected: the rowId in the linking-source section, in _that_ section's linking source, etc. + */ + linkingRowIds?: UIRowId[]; } export type ComponentKind = "safeBrowser" | "safePython" | "unsafeNode"; diff --git a/test/fixtures/docs/CursorWithRefLists1.grist b/test/fixtures/docs/CursorWithRefLists1.grist new file mode 100644 index 00000000..ec36547f Binary files /dev/null and b/test/fixtures/docs/CursorWithRefLists1.grist differ diff --git a/test/nbrowser/CursorSaving.ts b/test/nbrowser/CursorSaving.ts new file mode 100644 index 00000000..40321817 --- /dev/null +++ b/test/nbrowser/CursorSaving.ts @@ -0,0 +1,223 @@ +import {assert, driver} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {cleanupExtraWindows, setupTestSuite} from 'test/nbrowser/testUtils'; + +describe('CursorSaving', function() { + this.timeout(20000); + cleanupExtraWindows(); + const cleanup = setupTestSuite(); + const clipboard = gu.getLockableClipboard(); + afterEach(() => gu.checkForErrors()); + + describe('WithRefLists', function() { + before(async function() { + const session = await gu.session().login(); + await session.tempDoc(cleanup, "CursorWithRefLists1.grist"); + }); + + it('should remember positions when record is linked from multiple source records', async function() { + // Select Tag 'a' (row 1), and Item 'Apples' (row 1), which has tags 'b' and 'a'. + await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a'); + await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple'); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0}); + assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple'); + + // Now select a different Tag, but the same Item. + await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b'); + await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple'); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0}); + assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple'); + + // Try the third section. + await clickAndCheck({section: 'Items', rowNum: 3, col: 0}, 'Orange'); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange'); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 3, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange'); + + // Try getting to the same card via different selections. + await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a'); + await clickAndCheck({section: 'Items', rowNum: 2, col: 0}, 'Orange'); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange'); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 2, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange'); + }); + + it('should remember positions when "new" row is involved', async function() { + // Try a position when when the parent record is on a "new" row. + await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b'); + await clickAndCheck({section: 'Items', rowNum: 4, col: 0}, ''); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, ''); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 4, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), ''); + + // Try a position when when the grandparent parent record is on a "new" row. + await clickAndCheck({section: 'Tags', rowNum: 4, col: 0}, ''); + assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, ''); + + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 4, col: 0}); + assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), ''); + }); + + it('should create anchor links that preserve row positions in linking sources', async function() { + await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a'); + await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple'); + await gu.openRowMenu(1); + + const anchorLinks: string[] = []; + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // Now select a different Tag, but the same Item. + await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b'); + await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple'); + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // Try the third section. + await clickAndCheck({section: 'Items', rowNum: 3, col: 0}, 'Orange'); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange'); + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // A different way to get to the same value in third section. + await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a'); + await clickAndCheck({section: 'Items', rowNum: 2, col: 0}, 'Orange'); + await gu.getCardCell('Name', 'ITEMS Card').click(); + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // Now go through the anchor links, and make sure each gets us to the expected point. + await driver.get(anchorLinks[0]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0}); + assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple'); + + await driver.get(anchorLinks[1]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0}); + assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple'); + + await driver.get(anchorLinks[2]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 3, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange'); + + await driver.get(anchorLinks[3]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 2, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange'); + }); + + it('should handle anchor links when "new" row is involved', async function() { + const anchorLinks: string[] = []; + + // Try a position when when the parent record is on a "new" row. + await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b'); + await clickAndCheck({section: 'Items', rowNum: 4, col: 0}, ''); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, ''); + + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // Try a position when when the grandparent parent record is on a "new" row. + await clickAndCheck({section: 'Tags', rowNum: 4, col: 0}, ''); + assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/); + await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, ''); + + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + await driver.get(anchorLinks[0]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 4, col: 0}); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), ''); + + await driver.get(anchorLinks[1]); + assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 4, col: 0}); + assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/); + assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card'); + assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), ''); + }); + }); + + describe('WithRefs', function() { + // This is a similar test to the above, but without RefLists. In particular it checks that + // when a cursor is in the "new" row, enough is remembered to restore positions. + + before(async function() { + const session = await gu.session().login(); + const doc = await session.tempDoc(cleanup, "World.grist", {load: false}); + await session.loadDoc(`/doc/${doc.id}/p/5`, {wait: true}); + }); + + it('should remember row positions in linked sections', async function() { + // Select a country and a city within it. + await clickAndCheck({section: 'Country', rowNum: 2, col: 0}, 'AFG'); + await clickAndCheck({section: 'City', rowNum: 4, col: 1}, 'Balkh'); + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 4, col: 1}); + + // Now select a country, and the "new" row in the linked City widget. + await clickAndCheck({section: 'Country', rowNum: 3, col: 0}, 'AGO'); + await clickAndCheck({section: 'City', rowNum: 6, col: 1}, ''); + await gu.reloadDoc(); + assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 3, col: 0}); + assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 6, col: 1}); + }); + + it('should create anchor links that preserve row positions in linked sections', async function() { + const anchorLinks: string[] = []; + + // Select a country and a city within it. + await clickAndCheck({section: 'Country', rowNum: 2, col: 0}, 'AFG'); + await clickAndCheck({section: 'City', rowNum: 4, col: 1}, 'Balkh'); + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + // Now select a country, and the "new" row in the linked City widget. + await clickAndCheck({section: 'Country', rowNum: 3, col: 0}, 'AGO'); + await clickAndCheck({section: 'City', rowNum: 6, col: 1}, ''); + + await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); }); + + await driver.get(anchorLinks[0]); + assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 2, col: 0}); + assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 4, col: 1}); + + await driver.get(anchorLinks[1]); + assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 3, col: 0}); + assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 6, col: 1}); + }); + }); +}); + +async function clickAndCheck(options: gu.ICellSelect, expectedValue: string) { + const cell = gu.getCell(options); + await cell.click(); + assert.equal(await cell.getText(), expectedValue); +} + +async function clickAndCheckCard(options: gu.ICellSelect, expectedValue: string) { + const cell = gu.getDetailCell(options); + await cell.click(); + assert.equal(await cell.getText(), expectedValue); +}