(core) Highlight rows used as a selector in linking, but do not show 'inactive' cursors.

Summary:
1. Introduces another highlight for link-selector rows, with the same color as
   regular selection, and allowing to overlap with regular selection.
2. Don't show "secondary" cursors (those in inactive sections), to keep a single
   cursor on the screen, since having multiple (which different in color) could
   cause confusion.
3. An unrelated improvement (prompted by a new fixture doc) is to default the
   active section to the top-left one (rather than the one with smallest rowId).
4. Another unrelated improvement (prompted by a test affected by the previous unrelated improvement) is to skip chart widgets when searching (previously search would step through those with an invisible "cursor").

Includes also tweaks for better testing on Arm-based Macs:
- Add support for TEST_CHROME_BINARY_PATH environment variable (helpful for a Mac arm64 architecture workaround)
- Remove unsetting of SELENIUM_REMOTE_URL when running headless (unlikely to affect anyone, and can be done outside the script, but interferes with the Mac workaround)

Test Plan: Added a new test case that cursor and linking-selector CSS classes are present or absent appropriately. Fixed test affected by the fix to default active section.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3891
This commit is contained in:
Dmitry S 2023-06-21 10:43:22 -04:00
parent 7e50467396
commit 3fa5125cf7
11 changed files with 146 additions and 12 deletions

View File

@ -147,6 +147,8 @@ function BaseView(gristDoc, viewSectionModel, options) {
} }
})); }));
this.isLinkSource = this.autoDispose(ko.pureComputed(() => this.viewSection.linkedSections().all().length > 0));
// Indicated whether editing the section should be disabled given the current linking state. // Indicated whether editing the section should be disabled given the current linking state.
this.disableEditing = this.autoDispose(ko.computed(() => { this.disableEditing = this.autoDispose(ko.computed(() => {
const linking = this.viewSection.linkingState(); const linking = this.viewSection.linkingState();

View File

@ -1226,6 +1226,8 @@ GridView.prototype.buildDom = function() {
dom.autoDispose(fontUnderline), dom.autoDispose(fontUnderline),
dom.autoDispose(fontStrikethrough), dom.autoDispose(fontStrikethrough),
kd.toggleClass('link_selector_row', () => self.isLinkSource() && isRowActive()),
// rowid dom // rowid dom
dom('div.gridview_data_row_num', dom('div.gridview_data_row_num',
kd.style("width", ROW_NUMBER_WIDTH + 'px'), kd.style("width", ROW_NUMBER_WIDTH + 'px'),

View File

@ -172,15 +172,14 @@
top: 0px; top: 0px;
width: 100%; width: 100%;
height: 100%; height: 100%;
/* one pixel outline around the cell, and one inside the cell */
outline: 1px solid var(--grist-theme-cursor-inactive, var(--grist-color-inactive-cursor));
box-shadow: inset 0 0 0 1px var(--grist-theme-cursor-inactive, var(--grist-color-inactive-cursor));
pointer-events: none; pointer-events: none;
} }
.active_cursor { .active_cursor {
/* one pixel outline around the cell, and one inside the cell */
outline: 1px solid var(--grist-theme-cursor, var(--grist-color-cursor)); outline: 1px solid var(--grist-theme-cursor, var(--grist-color-cursor));
box-shadow: inset 0 0 0 1px var(--grist-theme-cursor, var(--grist-color-cursor)); box-shadow: inset 0 0 0 1px var(--grist-theme-cursor, var(--grist-color-cursor));
z-index: 1;
} }
} }
@ -207,6 +206,22 @@
background-color: var(--grist-theme-table-header-selected-bg, var(--grist-color-medium-grey-opaque)); background-color: var(--grist-theme-table-header-selected-bg, var(--grist-color-medium-grey-opaque));
} }
.link_selector_row > .gridview_data_row_num {
color: var(--grist-theme-left-panel-active-page-fg, white);
background-color: var(--grist-theme-left-panel-active-page-bg, var(--grist-color-dark-bg));
}
.link_selector_row > .record::after {
content: "";
position: absolute;
inset: 0;
pointer-events: none;
background-color: var(--grist-theme-selection, var(--grist-color-selection));
/* z-index should be higher than '.record .field.frozen' (10) to show for frozen columns,
* but lower than '.gridview_stick-top' (20) to stay under column headers. */
z-index: 15;
}
.gridview_data_row_info.linked_dst::before { .gridview_data_row_info.linked_dst::before {
position: absolute; position: absolute;
content: '\25B8'; content: '\25B8';

View File

@ -314,14 +314,18 @@ class FinderImpl implements IFinder {
private _initNewSectionShown() { private _initNewSectionShown() {
this._initNewSectionCommon(); this._initNewSectionCommon();
const viewInstance = this._sectionStepper.value.viewInstance.peek()!; const viewInstance = this._sectionStepper.value.viewInstance.peek()!;
this._rowStepper.array = viewInstance.sortedRows.getKoArray().peek() as number[]; const skip = ['chart'].includes(this._sectionStepper.value.parentKey.peek());
this._rowStepper.array = skip ? [] : viewInstance.sortedRows.getKoArray().peek() as number[];
} }
private async _initNewSectionAny() { private async _initNewSectionAny() {
const tableModel = this._initNewSectionCommon(); const tableModel = this._initNewSectionCommon();
const viewInstance = this._sectionStepper.value.viewInstance.peek(); const viewInstance = this._sectionStepper.value.viewInstance.peek();
if (viewInstance) { const skip = ['chart'].includes(this._sectionStepper.value.parentKey.peek());
if (skip) {
this._rowStepper.array = [];
} else if (viewInstance) {
this._rowStepper.array = viewInstance.sortedRows.getKoArray().peek() as number[]; this._rowStepper.array = viewInstance.sortedRows.getKoArray().peek() as number[];
} else { } else {
// If we are searching through another page (not currently loaded), we will NOT have a // If we are searching through another page (not currently loaded), we will NOT have a

View File

@ -59,13 +59,15 @@ export function createViewRec(this: ViewRec, docModel: DocModel): void {
const collapsed = new Set(this.activeCollapsedSections()); const collapsed = new Set(this.activeCollapsedSections());
const visible = all.filter(x => !collapsed.has(x.id())); const visible = all.filter(x => !collapsed.has(x.id()));
return visible.length > 0 ? visible[0].getRowId() : 0; // Default to the first leaf from layoutSpec (which corresponds to the top-left section), or
// fall back to the first item in the list if anything goes wrong (previous behavior).
const firstLeaf = getFirstLeaf(this.layoutSpecObj.peek());
return visible.find(s => s.getRowId() === firstLeaf) ? firstLeaf as number :
(visible[0]?.getRowId() || 0);
}); });
this.activeSection = refRecord(docModel.viewSections, this.activeSectionId); this.activeSection = refRecord(docModel.viewSections, this.activeSectionId);
// If the active section is removed, set the next active section to be the default. // If the active section is removed, set the next active section to be the default.
this._isActiveSectionGone = this.autoDispose(ko.computed(() => this.activeSection()._isDeleted())); this._isActiveSectionGone = this.autoDispose(ko.computed(() => this.activeSection()._isDeleted()));
this.autoDispose(this._isActiveSectionGone.subscribe(gone => { this.autoDispose(this._isActiveSectionGone.subscribe(gone => {
@ -74,3 +76,10 @@ export function createViewRec(this: ViewRec, docModel: DocModel): void {
} }
})); }));
} }
function getFirstLeaf(layoutSpec: BoxSpec|undefined): BoxSpec['leaf'] {
while (layoutSpec?.children?.length) {
layoutSpec = layoutSpec.children[0];
}
return layoutSpec?.leaf;
}

View File

@ -35,6 +35,9 @@ import defaults = require('lodash/defaults');
export interface ViewSectionRec extends IRowModel<"_grist_Views_section">, RuleOwner { export interface ViewSectionRec extends IRowModel<"_grist_Views_section">, RuleOwner {
viewFields: ko.Computed<KoArray<ViewFieldRec>>; viewFields: ko.Computed<KoArray<ViewFieldRec>>;
// List of sections linked from this one, i.e. for whom this one is the selector or link source.
linkedSections: ko.Computed<KoArray<ViewSectionRec>>;
// All table columns associated with this view section, excluding hidden helper columns. // All table columns associated with this view section, excluding hidden helper columns.
columns: ko.Computed<ColumnRec[]>; columns: ko.Computed<ColumnRec[]>;
@ -273,6 +276,7 @@ export interface Filter {
export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel): void { export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel): void {
this.viewFields = recordSet(this, docModel.viewFields, 'parentId', {sortBy: 'parentPos'}); this.viewFields = recordSet(this, docModel.viewFields, 'parentId', {sortBy: 'parentPos'});
this.linkedSections = recordSet(this, docModel.viewSections, 'linkSrcSectionRef');
// All table columns associated with this view section, excluding any hidden helper columns. // All table columns associated with this view section, excluding any hidden helper columns.
this.columns = this.autoDispose(ko.pureComputed(() => this.table().columns().all().filter(c => !c.isHiddenCol()))); this.columns = this.autoDispose(ko.pureComputed(() => this.table().columns().all().filter(c => !c.isHiddenCol())));

View File

@ -549,9 +549,7 @@ export class RightPanel extends Disposable {
]), ]),
domComputed((use) => { domComputed((use) => {
const activeSectionRef = activeSection.getRowId(); const selectorFor = use(use(activeSection.linkedSections).getObservable());
const allViewSections = use(use(viewModel.viewSections).getObservable());
const selectorFor = allViewSections.filter((sec) => use(sec.linkSrcSectionRef) === activeSectionRef);
// TODO: sections should be listed following the order of appearance in the view layout (ie: // TODO: sections should be listed following the order of appearance in the view layout (ie:
// left/right - top/bottom); // left/right - top/bottom);
return selectorFor.length ? [ return selectorFor.length ? [

Binary file not shown.

View File

@ -0,0 +1,96 @@
import {assert, WebElement} from 'mocha-webdriver';
import * as gu from 'test/nbrowser/gristUtils';
import {setupTestSuite} from 'test/nbrowser/testUtils';
describe('LinkingSelector', function() {
this.timeout(20000);
const cleanup = setupTestSuite({team: true});
let session: gu.Session;
afterEach(() => gu.checkForErrors());
before(async function() {
session = await gu.session().login();
const doc = await session.tempDoc(cleanup, 'Class Enrollment.grist', {load: false});
await session.loadDoc(`/doc/${doc.id}/p/7`);
});
interface CursorSelectorInfo {
linkSelector: false | number;
cursor: false | {rowNum: number, col: number};
}
async function getCursorSelectorInfo(section: WebElement): Promise<CursorSelectorInfo> {
const hasCursor = await section.find('.active_cursor').isPresent();
const hasSelector = await section.find('.link_selector_row').isPresent();
return {
linkSelector: hasSelector && Number(await section.find('.link_selector_row .gridview_data_row_num').getText()),
cursor: hasCursor && await gu.getCursorPosition(section),
};
}
it('should mark selected row used for linking', async function() {
const families = gu.getSection('FAMILIES');
const students = gu.getSection('STUDENTS');
const enrollments = gu.getSection('ENROLLMENTS');
// Initially FAMILIES first row should be selected and marked as selector.
assert.deepEqual(await getCursorSelectorInfo(families), {linkSelector: 1, cursor: {rowNum: 1, col: 0}});
assert.deepEqual(await gu.getActiveCell().getText(), 'Fin');
// STUDENTS shows appropriate records.
assert.deepEqual(await gu.getVisibleGridCells({section: students, col: 'First_Name', rowNums: [1, 2, 3, 4]}),
['Brockie', 'Care', 'Alfonso', '']);
// STUDENTS also has a selector row, but no active cursor.
assert.deepEqual(await getCursorSelectorInfo(students), {linkSelector: 1, cursor: false});
assert.deepEqual(await getCursorSelectorInfo(enrollments), {linkSelector: false, cursor: false});
// Select a different Family
await gu.getCell({section: families, rowNum: 3, col: 'First_Name'}).click();
assert.deepEqual(await gu.getActiveCell().getText(), 'Pat');
assert.deepEqual(await getCursorSelectorInfo(families), {linkSelector: 3, cursor: {rowNum: 3, col: 0}});
// STUDENTS shows new values, has a new selector row
assert.deepEqual(await gu.getVisibleGridCells({section: students, col: 'First_Name', rowNums: [1, 2, 3]}),
['Mordy', 'Noam', '']);
assert.deepEqual(await getCursorSelectorInfo(students), {linkSelector: 1, cursor: false});
// STUDENTS Card shows appropriate value
assert.deepEqual(await gu.getVisibleDetailCells(
{section: 'STUDENTS Card', cols: ['First_Name', 'Policy_Number'], rowNums: [1]}),
['Mordy', '468617']);
// Select another student
await gu.getCell({section: students, rowNum: 2, col: 'Last_Name'}).click();
assert.deepEqual(await getCursorSelectorInfo(students), {linkSelector: 2, cursor: {rowNum: 2, col: 1}});
assert.deepEqual(await gu.getVisibleDetailCells(
{section: 'STUDENTS Card', cols: ['First_Name', 'Policy_Number'], rowNums: [1]}),
['Noam', '663208']);
// There is no longer a cursor in FAMILIES, but still a link-selector.
assert.deepEqual(await getCursorSelectorInfo(families), {linkSelector: 3, cursor: false});
// Enrollments is linked to the selected student, but still shows no cursor or selector.
assert.deepEqual(await getCursorSelectorInfo(enrollments), {linkSelector: false, cursor: false});
assert.deepEqual(await gu.getVisibleGridCells({section: enrollments, col: 'Class', rowNums: [1, 2, 3]}),
['2019F-Yoga', '2019S-Yoga', '']);
// Click into an enrollment; it will become the only section with a cursor.
await gu.getCell({section: enrollments, rowNum: 2, col: 'Status'}).click();
assert.deepEqual(await getCursorSelectorInfo(enrollments), {linkSelector: false, cursor: {rowNum: 2, col: 2}});
assert.deepEqual(await getCursorSelectorInfo(students), {linkSelector: 2, cursor: false});
assert.deepEqual(await getCursorSelectorInfo(families), {linkSelector: 3, cursor: false});
});
it('should show correct state on reload after cursors are positioned', async function() {
await gu.reloadDoc();
const families = gu.getSection('FAMILIES');
const students = gu.getSection('STUDENTS');
const enrollments = gu.getSection('ENROLLMENTS');
assert.deepEqual(await getCursorSelectorInfo(enrollments), {linkSelector: false, cursor: {rowNum: 2, col: 2}});
assert.deepEqual(await getCursorSelectorInfo(students), {linkSelector: 2, cursor: false});
assert.deepEqual(await getCursorSelectorInfo(families), {linkSelector: 3, cursor: false});
});
});

View File

@ -358,7 +358,7 @@ export async function getVisibleGridCellsFast(colOrOptions: any, rowNums?: numbe
* If rowNums are not shown (for single-card view), use rowNum of 1. * If rowNums are not shown (for single-card view), use rowNum of 1.
*/ */
export async function getVisibleDetailCells(col: number|string, rows: number[], section?: string): Promise<string[]>; export async function getVisibleDetailCells(col: number|string, rows: number[], section?: string): Promise<string[]>;
export async function getVisibleDetailCells<T = string>(options: IColSelect<T>): Promise<T[]>; export async function getVisibleDetailCells<T = string>(options: IColSelect<T>|IColsSelect<T>): Promise<T[]>;
export async function getVisibleDetailCells<T>( export async function getVisibleDetailCells<T>(
colOrOptions: number|string|IColSelect<T>|IColsSelect<T>, _rowNums?: number[], _section?: string colOrOptions: number|string|IColSelect<T>|IColsSelect<T>, _rowNums?: number[], _section?: string
): Promise<T[]> { ): Promise<T[]> {

View File

@ -25,6 +25,10 @@ import {server} from 'test/nbrowser/testServer';
export {server}; export {server};
setOptionsModifyFunc(({chromeOpts, firefoxOpts}) => { setOptionsModifyFunc(({chromeOpts, firefoxOpts}) => {
if (process.env.TEST_CHROME_BINARY_PATH) {
chromeOpts.setChromeBinaryPath(process.env.TEST_CHROME_BINARY_PATH);
}
// Set "kiosk" printing that saves to PDF without offering any dialogs. This applies to regular // Set "kiosk" printing that saves to PDF without offering any dialogs. This applies to regular
// (non-headless) Chrome. On headless Chrome, no dialog or output occurs regardless. // (non-headless) Chrome. On headless Chrome, no dialog or output occurs regardless.
chromeOpts.addArguments("--kiosk-printing"); chromeOpts.addArguments("--kiosk-printing");