diff --git a/app/client/components/LinkingState.ts b/app/client/components/LinkingState.ts index 11aa8cc6..5cdddf64 100644 --- a/app/client/components/LinkingState.ts +++ b/app/client/components/LinkingState.ts @@ -103,7 +103,16 @@ export class LinkingState extends Disposable { // TODO: This approach doesn't help cursor-linking (the other direction). If we have the // inverse of summary-table's 'group' column, we could implement both, and more efficiently. const isDirectSummary = srcSection.table().summarySourceTable() === tgtSection.table().getRowId(); - this.filterColValues = this.autoDispose(ko.computed(() => { + const _filterColValues = ko.observable(); + this.filterColValues = this.autoDispose(ko.computed(() => _filterColValues())); + + // source data table could still be loading (this could happen after changing the group by + // columns of a linked summary table for instance), hence the below listeners. + this.autoDispose(srcTableData.dataLoadedEmitter.addListener(_update)); + this.autoDispose(srcTableData.tableActionEmitter.addListener(_update)); + + _update(); + function _update() { const result: FilterColValues = {filters: {}, operations: {}}; const srcRowId = srcSection.activeRowId(); for (const c of srcSection.table().groupByColumns()) { @@ -119,8 +128,8 @@ export class LinkingState extends Disposable { } } } - return result; - })); + _filterColValues(result); + } } else if (isSummaryOf(tgtSection.table(), srcSection.table())) { // TODO: We should move the cursor, but don't currently it for summaries. For that, we need a // column or map representing the inverse of summary table's "group" column. diff --git a/app/client/models/entities/ViewSectionRec.ts b/app/client/models/entities/ViewSectionRec.ts index 1e60ff16..860a3554 100644 --- a/app/client/models/entities/ViewSectionRec.ts +++ b/app/client/models/entities/ViewSectionRec.ts @@ -22,7 +22,7 @@ import {arrayRepeat} from 'app/common/gutil'; import {Sort} from 'app/common/SortSpec'; import {ColumnsToMap, WidgetColumnMap} from 'app/plugin/CustomSectionAPI'; import {ColumnToMapImpl} from 'app/client/models/ColumnToMap'; -import {Computed, Observable} from 'grainjs'; +import {Computed, Holder, Observable} from 'grainjs'; import * as ko from 'knockout'; import defaults = require('lodash/defaults'); @@ -118,6 +118,7 @@ export interface ViewSectionRec extends IRowModel<"_grist_Views_section"> { // Linking state maintains .filterFunc and .cursorPos observables which we use for // auto-scrolling and filtering. linkingState: ko.Computed; + _linkingState: Holder; // Holder for the current value of linkingState linkingFilter: ko.Computed; @@ -473,15 +474,14 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel): this.activeRowId = ko.observable(null); + this._linkingState = Holder.create(this); this.linkingState = this.autoDispose(ko.pureComputed(() => { - if (!this.linkSrcSection().getRowId()) { - return null; - } try { const config = new LinkConfig(this); - return new LinkingState(docModel, config); + return LinkingState.create(this._linkingState, docModel, config); } catch (err) { - console.warn(`Can't create LinkingState: ${err.message}`); + // Dispose old LinkingState in case creating the new one failed. + this._linkingState.dispose(); return null; } })); diff --git a/app/client/ui/selectBy.ts b/app/client/ui/selectBy.ts index ce4675d7..00be302b 100644 --- a/app/client/ui/selectBy.ts +++ b/app/client/ui/selectBy.ts @@ -257,13 +257,14 @@ export class LinkConfig { // Use null for unset cols (rather than an empty ColumnRec) for easier comparisons below. const srcCol = this.srcCol?.getRowId() ? this.srcCol : null; const tgtCol = this.tgtCol?.getRowId() ? this.tgtCol : null; - const srcTableId = (srcCol ? getReferencedTableId(srcCol.type.peek()) : - this.srcSection.table.peek().primaryTableId.peek()); - const tgtTableId = (tgtCol ? getReferencedTableId(tgtCol.type.peek()) : - this.tgtSection.table.peek().primaryTableId.peek()); + const srcTableId = (srcCol ? getReferencedTableId(srcCol.type()) : + this.srcSection.table().primaryTableId()); + const tgtTableId = (tgtCol ? getReferencedTableId(tgtCol.type()) : + this.tgtSection.table().primaryTableId()); try { - assert(!tgtCol || tgtCol.parentId.peek() === this.tgtSection.tableRef.peek(), "tgtCol belongs to wrong table"); - assert(!srcCol || srcCol.parentId.peek() === this.srcSection.tableRef.peek(), "srcCol belongs to wrong table"); + assert(Boolean(this.srcSection.getRowId()), "srcSection was disposed"); + assert(!tgtCol || tgtCol.parentId() === this.tgtSection.tableRef(), "tgtCol belongs to wrong table"); + assert(!srcCol || srcCol.parentId() === this.srcSection.tableRef(), "srcCol belongs to wrong table"); assert(this.srcSection.getRowId() !== this.tgtSection.getRowId(), "srcSection links to itself"); assert(tgtTableId, "tgtCol not a valid reference"); assert(srcTableId, "srcCol not a valid reference"); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index cb07ae98..bef64ae9 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -331,8 +331,9 @@ export function getActiveCell(): WebElementPromise { * Get the numeric value from the row header of the first selected row. This would correspond to * the row with the cursor when a single rows is selected. */ -export async function getSelectedRowNum(): Promise { - const rowNum = await driver.find('.active_section .gridview_data_row_num.selected').getText(); +export async function getSelectedRowNum(section?: string): Promise { + const sectionElem = section ? await getSection(section) : await driver.find('.active_section'); + const rowNum = await sectionElem.find('.gridview_data_row_num.selected').getText(); return parseInt(rowNum, 10); } @@ -805,7 +806,7 @@ export async function addNewTable() { export interface PageWidgetPickerOptions { summarize?: RegExp[]; // Optional list of patterns to match Group By columns. - selectBy?: RegExp; // Optional pattern of SELECT BY option to pick. + selectBy?: RegExp|string; // Optional pattern of SELECT BY option to pick. } // Add a new page using the 'Add New' menu and wait for the new page to be shown. @@ -847,14 +848,18 @@ export async function selectWidget(typeRe: RegExp, tableRe: RegExp, options: Pag // let's select table await tableEl.click(); + const pivotEl = tableEl.find('.test-wselect-pivot'); + if (await pivotEl.isPresent()) { + await toggleSelectable(pivotEl, Boolean(options.summarize)); + } if (options.summarize) { - // if summarize is requested, let's select the corresponding pivot icon - await tableEl.find('.test-wselect-pivot').click(); - - // and all the columns - for (const colRef of options.summarize) { - await driver.findContent('.test-wselect-column', colRef).click(); + for (const columnEl of await driver.findAll('.test-wselect-column')) { + const label = await columnEl.getText(); + // TODO: Matching cols with regexp calls for trouble and adds no value. I think function should be + // rewritten using string matching only. + const goal = Boolean(options.summarize.find(r => label.match(r))); + await toggleSelectable(columnEl, goal); } } @@ -870,6 +875,17 @@ export async function selectWidget(typeRe: RegExp, tableRe: RegExp, options: Pag await waitForServer(); } +/** + * Toggle elem if not selected. Expects elem to be clickable and to have a class ending with + * -selected when selected. + */ +async function toggleSelectable(elem: WebElement, goal: boolean) { + const isSelected = await elem.matches('[class*=-selected]'); + if (goal !== isSelected) { + await elem.click(); + } +} + /** * Rename the given page to a new name. The oldName can be a full string name or a RegExp. */