(core) Add shortcut for opening Record Card

Summary: Also adds tests for previously untested Record Card behavior.

Test Plan: Browser tests.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D4136
This commit is contained in:
George Gevoian 2023-12-08 00:59:42 -05:00
parent d0318af39b
commit 7e05284cf2
12 changed files with 376 additions and 21 deletions

View File

@ -228,7 +228,7 @@ BaseView.commonCommands = {
this.scrollToCursor(true).catch(reportError);
this.activateEditorAtCursor({init});
},
editField: function() { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor(); },
editField: function(event) { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor({event}); },
insertRecordBefore: function() { this.insertRow(this.cursor.rowIndex()); },
insertRecordAfter: function() { this.insertRow(this.cursor.rowIndex() + 1); },
@ -243,6 +243,12 @@ BaseView.commonCommands = {
filterByThisCellValue: function() { this.filterByThisCellValue(); },
duplicateRows: function() { this._duplicateRows().catch(reportError); },
openDiscussion: function() { this.openDiscussionAtCursor(); },
viewAsCard: function() {
/* Overridden by subclasses.
*
* This is still needed so that <space> doesn't trigger the `input` command
* if a subclass doesn't support opening the current record as a card. */
},
};
BaseView.prototype.selectedRows = function() {

View File

@ -308,7 +308,7 @@ GridView.gridCommands = {
fieldEditSave: function() { this.cursor.rowIndex(this.cursor.rowIndex() + 1); },
// Re-define editField after fieldEditSave to make it take precedence for the Enter key.
editField: function() { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor(); },
editField: function(event) { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor({event}); },
insertFieldBefore: function(maybeKeyboardEvent) {
if (!maybeKeyboardEvent) {

View File

@ -71,7 +71,6 @@ export type CommandName =
| 'input'
| 'editLabel'
| 'editLayout'
| 'toggleCheckbox'
| 'historyPrevious'
| 'historyNext'
| 'makeFormula'
@ -273,7 +272,7 @@ export const groups: CommendGroupDef[] = [{
},
{
name: 'viewAsCard',
keys: [],
keys: ['Space'],
desc: 'Show the record card widget of the selected record',
},
]
@ -483,10 +482,6 @@ export const groups: CommendGroupDef[] = [{
name: 'editLayout',
keys: [],
desc: 'Edit record layout'
}, {
name: 'toggleCheckbox',
keys: ['Enter', 'Space'],
desc: 'Toggles the value of checkbox cells'
}, {
name: 'historyPrevious',
keys: ['Up'],

View File

@ -10,9 +10,10 @@ dispose.makeDisposable(CheckBoxEditor);
_.extend(CheckBoxEditor.prototype, TextEditor.prototype);
// For documentation, see NewBaseEditor.ts
CheckBoxEditor.skipEditor = function(typedVal, cellVal) {
if (typedVal === ' ') {
// This is a special case when user hits <space>. We return the toggled value to save, and by
CheckBoxEditor.skipEditor = function(typedVal, cellVal, {event}) {
// eslint-disable-next-line no-undef
if (typedVal === '<enter>' || (event && event instanceof KeyboardEvent)) {
// This is a special case when user hits <enter>. We return the toggled value to save, and by
// this indicate that the editor should not open.
return !cellVal;
}

View File

@ -699,6 +699,7 @@ export class FieldBuilder extends Disposable {
public buildEditorDom(editRow: DataRowModel, mainRowModel: DataRowModel, options: {
init?: string,
state?: any
event?: KeyboardEvent | MouseEvent
}) {
// If the user attempts to edit a value during transform, finalize (i.e. cancel or execute)
// the transform.
@ -733,7 +734,13 @@ export class FieldBuilder extends Disposable {
return clearOwn();
}
if (!this._readonly.get() && saveWithoutEditor(editorCtor, editRow, this.field, options.init)) {
if (
!this._readonly.get() &&
saveWithoutEditor(editorCtor, editRow, this.field, {
typedVal: options.init,
event: options.event,
})
) {
return clearOwn();
}

View File

@ -24,16 +24,20 @@ const t = makeT('FieldEditor');
/**
* Check if the typed-in value should change the cell without opening the cell editor, and if so,
* saves and returns true. E.g. on typing space, CheckBoxEditor toggles the cell without opening.
* saves and returns true. E.g. on typing enter, CheckBoxEditor toggles the cell without opening.
*/
export function saveWithoutEditor(
editorCtor: IEditorConstructor, editRow: DataRowModel, field: ViewFieldRec, typedVal: string|undefined
editorCtor: IEditorConstructor,
editRow: DataRowModel,
field: ViewFieldRec,
options: {typedVal?: string, event?: KeyboardEvent|MouseEvent}
): boolean {
const {typedVal, event} = options;
// Never skip the editor if editing a formula. Also, check that skipEditor static function
// exists (we don't bother adding it on old-style JS editors that don't need it).
if (!field.column.peek().isRealFormula.peek() && editorCtor.skipEditor) {
const origVal = editRow.cells[field.colId()].peek();
const skipEditorValue = editorCtor.skipEditor(typedVal, origVal);
const skipEditorValue = editorCtor.skipEditor(typedVal, origVal, {event});
if (skipEditorValue !== undefined) {
setAndSave(editRow, field, skipEditorValue).catch(reportError);
return true;

View File

@ -54,9 +54,13 @@ export abstract class NewBaseEditor extends Disposable {
/**
* Check if the typed-in value should change the cell without opening the editor, and if so,
* returns the value to save. E.g. on typing " ", CheckBoxEditor toggles value without opening.
* returns the value to save. E.g. on typing <enter>, CheckBoxEditor toggles value without opening.
*/
public static skipEditor(typedVal: string|undefined, origVal: CellValue): CellValue|undefined {
public static skipEditor(
typedVal: string|undefined,
origVal: CellValue,
options?: {event?: KeyboardEvent|MouseEvent}
): CellValue|undefined {
return undefined;
}

View File

@ -86,6 +86,7 @@ export class ReferenceList extends Reference {
ev.stopPropagation();
ev.preventDefault();
}),
testId('ref-list-link-icon'),
),
cssLabel(isBlankReference ? '[Blank]' : formattedValue,
testId('ref-list-cell-token-label'),

View File

@ -18,11 +18,11 @@ abstract class ToggleBase extends NewAbstractWidget {
return;
}
if (!this.field.column().isRealFormula()) {
// Move the cursor here, and pretend that spacebar was pressed. This triggers an editing
// Move the cursor here, and pretend that enter was pressed. This triggers an editing
// flow which is handled by CheckBoxEditor.skipEditor(). This way the edit applies to
// editRow, which handles setting default values based on widget linking.
commands.allCommands.setCursor.run(row, this.field);
commands.allCommands.input.run(' ');
commands.allCommands.input.run('<enter>');
}
}),
dom.on('dblclick', (event) => {

View File

@ -448,7 +448,7 @@ describe('ChoiceList', function() {
it('should allow reasonable conversions between ChoiceList and other types', async function() {
await gu.enterGridRows({rowNum: 1, col: 'A'},
[['Hello'], ['World'], [' Foo,Bar;Baz!,"Qux, quux corge", "80\'s",']]);
[['Hello'], ['World'], ['Foo,Bar;Baz!,"Qux, quux corge", "80\'s",']]);
await testTextChoiceListConversions();
});
@ -534,7 +534,7 @@ describe('ChoiceList', function() {
assert.deepEqual(await gu.getVisibleGridCells({rowNums: [1, 2, 3], cols: ['A']}), [
'Hello',
'World',
' Foo,Bar;Baz!,"Qux, quux corge", "80\'s",', // That's the text originally entered into this Text cell.
'Foo,Bar;Baz!,"Qux, quux corge", "80\'s",', // That's the text originally entered into this Text cell.
]);
}

View File

@ -269,6 +269,27 @@ describe('RawData', function () {
]);
});
it('shows Record Card button for all non-summary tables', async function () {
const displayed = await getRawTableRecordCardButtonsIsDisplayed();
assert.deepEqual(displayed, [
true,
true,
true,
true,
false,
false,
]);
const enabled = await getRawTableRecordCardButtonsIsEnabled();
assert.deepEqual(enabled, [
true,
true,
true,
true,
false,
false,
]);
});
it('shows preview of summary table when clicked', async function () {
// Open a summary table.
await driver.findContent('.test-raw-data-table-title', 'CountryLanguage [by Country]').click();
@ -612,6 +633,139 @@ describe('RawData', function () {
await gu.checkForErrors();
await assertNoPopup();
});
it("can edit a table's Record Card", async () => {
// Open the Record Card for the Country table.
await openRawData();
await editRecordCard('Country');
// Check that the Record Card is shown.
assert.isTrue(await driver.findWait('.test-raw-data-overlay', 100).isDisplayed());
// Check that layout editing is toggled by default.
assert.isTrue(await driver.find('.test-edit-layout-controls').isDisplayed());
// Check that the title is correct. Note that it's initially obscured by the layout
// editing buttons; it becomes visible after the layout is saved.
assert.equal(await gu.getSectionTitle(), 'COUNTRY Card');
// Modify the layout and theme.
await gu.openWidgetPanel('widget');
assert.isTrue(
await driver.findContent('.active_section .g_record_detail_inner .g_record_detail_label',
gu.exactMatch('Continent')).isPresent()
);
await gu.moveToHidden('Continent');
assert.isFalse(
await driver.findContent('.active_section .g_record_detail_inner .g_record_detail_label',
gu.exactMatch('Continent')).isPresent()
);
await driver.findContent('.test-edit-layout-controls button', 'Save').click();
await gu.waitForServer();
await driver.find('.test-vconfigtab-detail-theme').click();
await driver.findContent('.test-select-row', /Blocks/).click();
await gu.waitForServer();
await gu.checkForErrors();
// Close the overlay.
await gu.sendKeys(Key.ESCAPE);
// Re-open the Record Card and check that the new layout and theme persisted.
await editRecordCard('Country');
assert.isFalse(
await driver.findContent('.active_section .g_record_detail_inner .g_record_detail_label',
gu.exactMatch('Continent')).isPresent()
);
assert.equal(
await driver.find('.test-vconfigtab-detail-theme').getText(),
'Blocks'
);
await gu.sendKeys(Key.ESCAPE, Key.ESCAPE);
// Open the Record Card from outside the Raw Data page and check that the
// new layout and theme is used.
await gu.openPage('Country');
await (await gu.openRowMenu(1)).findContent('li', /View as card/).click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.isFalse(
await driver.findContent('.active_section .g_record_detail_inner .g_record_detail_label',
gu.exactMatch('Continent')).isPresent()
);
assert.equal(
await driver.find('.test-vconfigtab-detail-theme').getText(),
'Blocks'
);
await gu.sendKeys(Key.ESCAPE);
});
it("can disable a table's Record Card", async () => {
// Disable the Record Card for the Country table.
await openRawData();
await disableRecordCard('Country');
// Check that the button to edit the Record Card is disabled.
assert.isFalse(await isRecordCardEnabled('Country'));
await editRecordCard('Country');
assert.isFalse(await driver.find('.test-raw-data-overlay').isPresent());
// Check that the Edit Record Card menu item still works though.
await openMenu('Country');
await driver.find('.test-raw-data-menu-edit-record-card').click();
assert.isTrue(await driver.findWait('.test-raw-data-overlay', 100).isDisplayed());
assert.equal(await gu.getSectionTitle(), 'COUNTRY Card');
// Stop editing the layout and close the overlay.
await gu.sendKeys(Key.ESCAPE, Key.ESCAPE);
// Check that it's no longer possible to open a Record Card from outside
// the Raw Data page, even with the keyboard shortcut.
await gu.openPage('Country');
await (await gu.openRowMenu(1)).findContent('li.disabled', /View as card/);
await gu.sendKeys(Key.ESCAPE, Key.SPACE);
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
// Check that clicking the icon in Reference and Reference List columns also
// doesn't open a Record Card.
await gu.openPage('CountryLanguage');
await gu.getCell(0, 1).find('.test-ref-link-icon').click();
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
await gu.setType('Reference List', {apply: true});
await gu.getCell(0, 1).find('.test-ref-list-link-icon').click();
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
});
it("can enable a table's Record Card", async () => {
// Enable the Record Card for the Country table.
await openRawData();
await enableRecordCard('Country');
// Check that the button to edit the Record Card is enabled again.
assert.isTrue(await isRecordCardEnabled('Country'));
await editRecordCard('Country');
assert.isTrue(await driver.findWait('.test-raw-data-overlay', 100).isDisplayed());
assert.equal(await gu.getSectionTitle(), 'COUNTRY Card');
// Check that it's possible again to open the Record Card from outside
// the Raw Data page.
await gu.openPage('Country');
await (await gu.openRowMenu(1)).findContent('li', /View as card/).click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
await gu.sendKeys(Key.ESCAPE);
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
await gu.sendKeys(Key.SPACE);
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
// Check that clicking the icon in Reference and Reference List columns opens a
// Record Card again.
await gu.openPage('CountryLanguage');
await gu.getCell(0, 1).find('.test-ref-list-link-icon').click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
await gu.sendKeys(Key.ESCAPE);
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
await gu.setType('Reference', {apply: true});
await gu.getCell(0, 1).find('.test-ref-link-icon').click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
});
});
const anchorRegex = /#a(\d+)\.s(\d+)\.r(\d+)\.c(\d+)/gm;
@ -695,6 +849,18 @@ async function getRawTableRows() {
return await driver.findAll('.test-raw-data-table-rows', e => e.getText());
}
async function getRawTableRecordCardButtonsIsDisplayed() {
return await driver.findAll('.test-raw-data-table-record-card', e => e.isDisplayed());
}
async function getRawTableRecordCardButtonsIsEnabled() {
return await driver.findAll('.test-raw-data-table-record-card', async e => {
const isDisplayed = await e.isDisplayed();
const className = await e.getAttribute('class');
return isDisplayed && !className.includes('-disabled');
});
}
async function openMenu(tableId: string) {
const allTables = await getRawTableIds();
const tableIndex = allTables.indexOf(tableId);
@ -716,6 +882,37 @@ async function isRemovable(tableId: string){
return disabledItems.length === 0;
}
async function editRecordCard(tableId: string, wait = true) {
await driver.findContent('.test-raw-data-table-title', tableId)
.findClosest('.test-raw-data-table')
.find('.test-raw-data-table-record-card')
.click();
if (wait) {
await gu.waitForServer();
}
}
async function disableRecordCard(tableId: string) {
await openMenu(tableId);
await driver.find('.test-raw-data-menu-disable-record-card').click();
await gu.waitForServer();
}
async function enableRecordCard(tableId: string) {
await openMenu(tableId);
await driver.find('.test-raw-data-menu-enable-record-card').click();
await gu.waitForServer();
}
async function isRecordCardEnabled(tableId: string) {
const recordCard = await driver.findContent('.test-raw-data-table-title', tableId)
.findClosest('.test-raw-data-table')
.find('.test-raw-data-table-record-card');
const isDisplayed = await recordCard.isDisplayed();
const className = await recordCard.getAttribute('class');
return isDisplayed && !className.includes('-disabled');
}
async function waitForPopup() {
assert.isTrue(await driver.findWait('.test-raw-data-overlay', 100).isDisplayed());
}

View File

@ -0,0 +1,140 @@
import {UserAPI} from 'app/common/UserAPI';
import {assert, driver, Key} from 'mocha-webdriver';
import * as gu from 'test/nbrowser/gristUtils';
import {setupTestSuite} from 'test/nbrowser/testUtils';
describe('RecordCards', function() {
this.timeout(30000);
let api: UserAPI;
let docId: string;
let session: gu.Session;
const cleanup = setupTestSuite();
before(async function() {
session = await gu.session().login();
docId = (await session.tempDoc(cleanup, 'World-v39.grist')).id;
api = session.createHomeApi();
await gu.openPage('Country');
});
afterEach(() => gu.checkForErrors());
describe('RowContextMenu', function() {
it('opens popup when keyboard shortcut is pressed', async function() {
await gu.sendKeys(Key.SPACE);
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'ALB');
assert.isFalse(await driver.find('.grist-single-record__menu').isPresent());
await gu.sendKeys(Key.ESCAPE);
});
it('opens popup when menu item is clicked', async function() {
await (await gu.openRowMenu(2)).findContent('li', /View as card/).click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'AND');
await gu.sendKeys(Key.ESCAPE);
});
it('closes popup when record is deleted', async function() {
await api.applyUserActions(docId, [
['RemoveRecord', 'Country', 1]
]);
await gu.waitToPass(async () => {
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
}, 2000);
await (await gu.openRowMenu(1)).findContent('li', /View as card/).click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
await gu.sendKeys(Key.chord(await gu.modKey(), Key.DELETE));
await driver.find('.test-confirm-save').click();
await gu.waitForServer();
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
});
it('hides option to open popup if more than 1 row is selected', async function() {
await gu.sendKeys(Key.chord(Key.SHIFT, Key.DOWN));
assert.isFalse(await (await gu.openRowMenu(1)).findContent('li', /View as card/).isPresent());
await gu.sendKeys(Key.ESCAPE, Key.SPACE);
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
});
it('disables option to open popup in "add new" row', async function() {
await gu.sendKeys(Key.chord(await gu.modKey(), Key.DOWN));
assert.isTrue(await (await gu.openRowMenu(120)).findContent('li.disabled', /View as card/).isPresent());
await gu.sendKeys(Key.ESCAPE, Key.SPACE);
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
});
});
describe('Reference', function() {
before(async function() {
await gu.openPage('CountryLanguage');
});
it('opens popup when reference icon is clicked', async function() {
await gu.getCell(0, 4).find('.test-ref-link-icon').click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'AFG');
assert.isFalse(await driver.find('.grist-single-record__menu').isPresent());
await gu.sendKeys(Key.ESCAPE);
});
it('updates popup when reference icon is clicked within Record Card popup', async function() {
await gu.getCell(0, 4).find('.test-ref-text').click();
await gu.sendKeys(Key.SPACE);
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRYLANGUAGE Card');
assert.equal(await gu.getCardCell('Country').getText(), 'AFG');
await gu.getCardCell('Country').find('.test-ref-link-icon').click();
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'AFG');
await gu.sendKeys(Key.ESCAPE);
});
it('does not open popup if cell is empty', async function() {
await gu.getCell(0, 4).find('.test-ref-text').click();
await driver.sendKeys(Key.DELETE);
await gu.waitForServer();
await gu.getCell(0, 4).find('.test-ref-link-icon').click();
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
await gu.undo();
});
it('does not open popup in "add new" row', async function() {
await gu.sendKeys(Key.chord(await gu.modKey(), Key.DOWN));
await gu.getCell(0, 747).find('.test-ref-link-icon').click();
assert.isFalse(await driver.find('.test-record-card-popup-overlay').isPresent());
});
});
describe('ReferenceList', function() {
before(async function() {
await gu.sendKeys(Key.chord(await gu.modKey(), Key.UP));
await gu.setType('Reference List', {apply: true});
});
it('opens popup when reference icon is clicked', async function() {
await gu.getCell(0, 4).find('.test-ref-list-link-icon').click();
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'AFG');
assert.isFalse(await driver.find('.grist-single-record__menu').isPresent());
await gu.sendKeys(Key.ESCAPE);
});
it('updates popup when reference icon is clicked within Record Card popup', async function() {
await gu.getCell(0, 4).click();
await gu.sendKeys(Key.SPACE);
assert.isTrue(await driver.findWait('.test-record-card-popup-overlay', 100).isDisplayed());
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRYLANGUAGE Card');
assert.equal(await gu.getCardCell('Country').getText(), 'AFG');
await gu.getCardCell('Country').find('.test-ref-list-link-icon').click();
assert.equal(await driver.find('.test-widget-title-text').getText(), 'COUNTRY Card');
assert.equal(await gu.getCardCell('Code').getText(), 'AFG');
await gu.sendKeys(Key.ESCAPE);
});
});
});