(core) Polish Importer UI

Summary:
Changes include:
 * Hide the colum matching section for new destinations (for now).
 * Make the preview table read-only.
 * Don't show helper column IDs when the formula editor is open.
 * Fix the formula editor autocomplete to show suggestions
 from the active transform section.
 * Hide the formula icons in the preview table, and other unnecessary
 UI elements such as row dropdown menus.
 * Keep preview loading spinner shown if scheduled (i.e. debounced) diff updates exist.

Test Plan: Browser tests.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3148
This commit is contained in:
George Gevoian 2021-11-18 21:35:01 -08:00
parent 7fe4423a6f
commit 32bb89235e
4 changed files with 121 additions and 87 deletions

View File

@ -30,6 +30,7 @@ const {encodeObject} = require("app/plugin/objtypes");
/**
* BaseView forms the basis for ViewSection classes.
* @param {Object} viewSectionModel - The model for the viewSection represented.
* @param {Boolean} options.isPreview - Whether the view is a read-only preview (e.g. Importer view).
* @param {Boolean} options.addNewRow - Whether to include an add row in the model.
*/
function BaseView(gristDoc, viewSectionModel, options) {
@ -168,6 +169,8 @@ function BaseView(gristDoc, viewSectionModel, options) {
return linking && linking.disableEditing();
}));
this.isPreview = this.options.isPreview;
this.enableAddRow = this.autoDispose(ko.computed(() => this.options.addNewRow &&
!this.viewSection.disableAddRemoveRows() && !this.disableEditing()));
@ -198,7 +201,9 @@ function BaseView(gristDoc, viewSectionModel, options) {
// A koArray of FieldBuilder objects, one for each view-section field.
this.fieldBuilders = this.autoDispose(
FieldBuilder.createAllFieldWidgets(this.gristDoc, this.viewSection.viewFields, this.cursor)
FieldBuilder.createAllFieldWidgets(this.gristDoc, this.viewSection.viewFields, this.cursor, {
isPreview: this.isPreview,
})
);
// An observable evaluating to the FieldBuilder for the field where the cursor is.

View File

@ -55,7 +55,7 @@ const ROW_NUMBER_WIDTH = 52;
* GridView component implements the view of a grid of cells.
*/
function GridView(gristDoc, viewSectionModel, isPreview = false) {
BaseView.call(this, gristDoc, viewSectionModel, { 'addNewRow': true });
BaseView.call(this, gristDoc, viewSectionModel, { isPreview, 'addNewRow': true });
this.viewSection = viewSectionModel;
@ -878,7 +878,7 @@ GridView.prototype.buildDom = function() {
kd.style('borderLeftWidth', v.borderWidthPx),
kd.foreach(v.viewFields(), field => {
var isEditingLabel = ko.pureComputed({
read: () => this.gristDoc.isReadonlyKo() ? false : editIndex() === field._index(),
read: () => this.gristDoc.isReadonlyKo() || self.isPreview ? false : editIndex() === field._index(),
write: val => editIndex(val ? field._index() : -1)
}).extend({ rateLimit: 0 });
let filterTriggerCtl;
@ -899,10 +899,10 @@ GridView.prototype.buildDom = function() {
if (btn) { btn.click(); }
}),
dom('div.g-column-label',
kf.editableLabel(field.displayLabel, isEditingLabel, renameCommands),
kf.editableLabel(self.isPreview ? field.label : field.displayLabel, isEditingLabel, renameCommands),
dom.on('mousedown', ev => isEditingLabel() ? ev.stopPropagation() : true)
),
this.isPreview ? null : menuToggle(null,
self.isPreview ? null : menuToggle(null,
kd.cssClass('g-column-main-menu'),
kd.cssClass('g-column-menu-btn'),
// Prevent mousedown on the dropdown triangle from initiating column drag.
@ -1001,7 +1001,7 @@ GridView.prototype.buildDom = function() {
ev.preventDefault();
ev.currentTarget.querySelector('.menu_toggle').click();
}),
menuToggle(null,
self.isPreview ? null : menuToggle(null,
dom.on('click', ev => self.maybeSelectRow(ev.currentTarget.parentNode, row.getRowId())),
menu(() => RowContextMenu({
disableInsert: Boolean(self.gristDoc.isReadonly.get() || self.viewSection.disableAddRemoveRows() || self.tableModel.tableMetaRow.onDemand()),
@ -1378,7 +1378,7 @@ GridView.prototype._getColumnMenuOptions = function(copySelection) {
numColumns: copySelection.fields.length,
numFrozen: this.viewSection.numFrozen.peek(),
disableModify: calcFieldsCondition(copySelection.fields, f => f.disableModify.peek()),
isReadonly: this.gristDoc.isReadonly.get(),
isReadonly: this.gristDoc.isReadonly.get() || this.isPreview,
isFiltered: this.isFiltered(),
isFormula: calcFieldsCondition(copySelection.fields, f => f.column.peek().isRealFormula.peek()),
};

View File

@ -170,7 +170,16 @@ export class Importer extends DisposableWithEvents {
// Promise for the most recent generateImportDiff action.
private _lastGenImportDiffPromise: Promise<any>|null = null;
private _updateImportDiff = debounce(this._updateDiff, 1000, {leading: true, trailing: true});
private _debouncedUpdateDiff = debounce(this._updateDiff, 1000, {leading: true, trailing: true});
/**
* Flag that is set when _updateImportDiff is called, and unset when _debouncedUpdateDiff begins executing.
*
* This is a workaround until Lodash's next release, which supports checking if a debounced function is
* pending. We need to know if more debounced calls are pending so that we can decide to take down the
* loading spinner over the preview table, or leave it up until all scheduled calls settle.
*/
private _hasScheduledDiffUpdate = false;
// destTables is a list of options for import destinations, and includes all tables in the
// document, plus two values: to import as a new table, and to skip an import table entirely.
@ -297,6 +306,9 @@ export class Importer extends DisposableWithEvents {
// Otherwise, update the transform section for `sourceInfo`.
sourceInfo.transformSection.set(this._gristDoc.docModel.viewSections.getRowModel(transformSectionRef));
sourceInfo.isLoadingSection.set(false);
// Change the active section to the transform section, so that formula autocomplete works.
this._gristDoc.viewModel.activeSectionId(transformSectionRef);
}
private _getTransformedDataSource(upload: UploadResult): DataSourceTransformed {
@ -479,16 +491,28 @@ export class Importer extends DisposableWithEvents {
* Triggers an update of the import diff in the preview table. When called in quick succession,
* only the most recent call will result in an update being made to the preview table.
*
* NOTE: This method should not be called directly. Instead, use _updateImportDiff, which
* wraps this method and debounces it.
* @param {SourceInfo} info The source to update the diff for.
*/
private async _updateImportDiff(info: SourceInfo) {
this._hasScheduledDiffUpdate = true;
this._isLoadingDiff.set(true);
await this._debouncedUpdateDiff(info);
}
/**
* NOTE: This method should not be called directly. Instead, use _updateImportDiff above, which
* wraps this method and calls a debounced version of it.
*
* Triggers an update of the import diff in the preview table. When called in quick succession,
* only the most recent call will result in an update being made to the preview table.
*
* @param {SourceInfo} info The source to update the diff for.
*/
private async _updateDiff(info: SourceInfo) {
// Reset the flag tracking scheduled updates since the debounced update has started.
this._hasScheduledDiffUpdate = false;
const mergeOptions = this._mergeOptions[info.hiddenTableId]!;
this._isLoadingDiff.set(true);
if (!mergeOptions.updateExistingRecords.get() || mergeOptions.mergeCols.get().length === 0) {
// We can simply disable document comparison mode when merging isn't configured.
this._gristDoc.comparison = null;
@ -506,7 +530,10 @@ export class Importer extends DisposableWithEvents {
this._gristDoc.comparison = diff;
}
this._isLoadingDiff.set(false);
// If more updates where scheduled since we started the update, leave the loading spinner up.
if (!this._hasScheduledDiffUpdate) {
this._isLoadingDiff.set(false);
}
}
/**
@ -523,8 +550,9 @@ export class Importer extends DisposableWithEvents {
* is functionally equivalent to canceling the outstanding requests.
*/
private _cancelPendingDiffRequests() {
this._updateImportDiff.cancel();
this._debouncedUpdateDiff.cancel();
this._lastGenImportDiffPromise = null;
this._hasScheduledDiffUpdate = false;
this._isLoadingDiff.set(false);
}
@ -579,14 +607,14 @@ export class Importer extends DisposableWithEvents {
const {mergeCols, updateExistingRecords, hasInvalidMergeCols} = this._mergeOptions[info.hiddenTableId]!;
return cssConfigAndPreview(
cssConfigColumn(
dom.maybe(info.transformSection, section => [
dom.maybe(info.destTableId, () => {
const updateRecordsListener = updateExistingRecords.addListener(async () => {
await this._updateImportDiff(info);
});
dom.maybe(info.destTableId, () => cssConfigColumn(
dom.maybe(info.transformSection, section => {
const updateRecordsListener = updateExistingRecords.addListener(async () => {
await this._updateImportDiff(info);
});
return cssMergeOptions(
return [
cssMergeOptions(
cssMergeOptionsToggle(labeledSquareCheckbox(
updateExistingRecords,
'Update existing records',
@ -619,77 +647,77 @@ export class Importer extends DisposableWithEvents {
)
];
})
);
}),
dom.domComputed(this._unmatchedFields, fields =>
fields && fields.length > 0 ?
cssUnmatchedFields(
dom('div',
cssGreenText(
`${fields.length} unmatched ${fields.length > 1 ? 'fields' : 'field'}`
),
dom.domComputed(this._unmatchedFields, fields =>
fields && fields.length > 0 ?
cssUnmatchedFields(
dom('div',
cssGreenText(
`${fields.length} unmatched ${fields.length > 1 ? 'fields' : 'field'}`
),
' in import:'
),
' in import:'
),
cssUnmatchedFieldsList(fields.join(', ')),
testId('importer-unmatched-fields')
) : null
),
cssColumnMatchOptions(
dom.forEach(fromKo(section.viewFields().getObservable()), field => cssColumnMatchRow(
cssColumnMatchIcon('ImportArrow'),
cssSourceAndDestination(
cssDestinationFieldRow(
cssDestinationFieldLabel(
dom.text(field.label),
),
cssDestinationFieldSettings(
icon('Dots'),
menu(
() => {
const sourceColId = field.origCol().id();
const sourceColIdsAndLabels = [...this._sourceColLabelsById.get()!.entries()];
return [
menuItem(
() => this._gristDoc.clearColumns([sourceColId]),
'Skip',
testId('importer-column-match-menu-item')
),
menuDivider(),
...sourceColIdsAndLabels.map(([id, label]) =>
cssUnmatchedFieldsList(fields.join(', ')),
testId('importer-unmatched-fields')
) : null
),
cssColumnMatchOptions(
dom.forEach(fromKo(section.viewFields().getObservable()), field => cssColumnMatchRow(
cssColumnMatchIcon('ImportArrow'),
cssSourceAndDestination(
cssDestinationFieldRow(
cssDestinationFieldLabel(
dom.text(field.label),
),
cssDestinationFieldSettings(
icon('Dots'),
menu(
() => {
const sourceColId = field.origCol().id();
const sourceColIdsAndLabels = [...this._sourceColLabelsById.get()!.entries()];
return [
menuItem(
() => this._setColumnFormula(sourceColId, '$' + id),
label,
() => this._gristDoc.clearColumns([sourceColId]),
'Skip',
testId('importer-column-match-menu-item')
),
),
testId('importer-column-match-menu'),
];
},
{placement: 'right-start'},
menuDivider(),
...sourceColIdsAndLabels.map(([id, label]) =>
menuItem(
() => this._setColumnFormula(sourceColId, '$' + id),
label,
testId('importer-column-match-menu-item')
),
),
testId('importer-column-match-menu'),
];
},
{ placement: 'right-start' },
),
testId('importer-column-match-destination-settings')
),
testId('importer-column-match-destination-settings')
testId('importer-column-match-destination')
),
testId('importer-column-match-destination')
),
dom.domComputed(use => dom.create(
this._buildColMappingFormula.bind(this),
use(field.column),
(elem: Element) => this._activateFormulaEditor(elem, field),
'Skip'
)),
testId('importer-column-match-source-destination'),
)
)),
testId('importer-column-match-options'),
)
]),
),
dom.domComputed(use => dom.create(
this._buildColMappingFormula.bind(this),
use(field.column),
(elem: Element) => this._activateFormulaEditor(elem, field),
'Skip'
)),
testId('importer-column-match-source-destination'),
)
)),
testId('importer-column-match-options'),
)
];
}),
)),
cssPreviewColumn(
cssSectionHeader('Preview'),
dom.domComputed(use => {
const previewSection = use(this._previewViewSection);
if (use(this._isLoadingDiff) || !previewSection) {
return cssPreviewSpinner(loadingSpinner());
return cssPreviewSpinner(loadingSpinner(), testId('importer-preview-spinner'));
}
const gridView = this._createPreview(previewSection);

View File

@ -36,10 +36,10 @@ const testId = makeTestId('test-fbuilder-');
// Creates a FieldBuilder object for each field in viewFields
export function createAllFieldWidgets(gristDoc: GristDoc, viewFields: ko.Computed<KoArray<ViewFieldRec>>,
cursor: Cursor) {
cursor: Cursor, options: { isPreview?: boolean } = {}) {
// TODO: Handle disposal from the map when fields are removed.
return viewFields().map(function(field) {
return new FieldBuilder(gristDoc, field, cursor);
return new FieldBuilder(gristDoc, field, cursor, options);
}).setAutoDisposeValues();
}
@ -80,7 +80,7 @@ export class FieldBuilder extends Disposable {
private readonly _readonly: Computed<boolean>;
public constructor(public readonly gristDoc: GristDoc, public readonly field: ViewFieldRec,
private _cursor: Cursor) {
private _cursor: Cursor, private _options: { isPreview?: boolean } = {}) {
super();
this._docModel = gristDoc.docModel;
@ -89,7 +89,8 @@ export class FieldBuilder extends Disposable {
this._readOnlyPureType = ko.pureComputed(() => this.field.column().pureType());
this._readonly = Computed.create(this, (use) => use(gristDoc.isReadonly) || use(field.disableEditData));
this._readonly = Computed.create(this, (use) =>
use(gristDoc.isReadonly) || use(field.disableEditData) || Boolean(this._options.isPreview));
// Observable with a list of available types.
this._availableTypes = Computed.create(this, (use) => {
@ -428,7 +429,7 @@ export class FieldBuilder extends Disposable {
this._rowMap.set(row, elem);
dom(elem,
dom.autoDispose(widgetObs),
kd.cssClass(this.field.formulaCssClass),
this._options.isPreview ? null : kd.cssClass(this.field.formulaCssClass),
kd.toggleClass("readonly", toKo(ko, this._readonly)),
kd.maybe(isSelected, () => dom('div.selected_cursor',
kd.toggleClass('active_cursor', isActive)