(core) Fix JS error when scrolling with a column of hyperlinks, and use stricter types.

Summary:
When scrolling quicly through a column with hyperlinks, null could be passed to
a function that didn't expect it. Added better types would help catch it.

Test Plan: Tested manually

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2813
pull/23/head
Dmitry S 3 years ago
parent d0d3d3d0c9
commit 28cb64f1f7

@ -5,7 +5,7 @@ import * as DataTableModel from 'app/client/models/DataTableModel';
import { IRowModel } from 'app/client/models/DocModel'; import { IRowModel } from 'app/client/models/DocModel';
import { ValidationRec } from 'app/client/models/entities/ValidationRec'; import { ValidationRec } from 'app/client/models/entities/ValidationRec';
import * as modelUtil from 'app/client/models/modelUtil'; import * as modelUtil from 'app/client/models/modelUtil';
import { ColValues } from 'app/common/DocActions'; import { CellValue, ColValues } from 'app/common/DocActions';
import * as ko from 'knockout'; import * as ko from 'knockout';
/** /**
@ -16,7 +16,7 @@ export class DataRowModel extends BaseRowModel {
// Instances of this class are indexable, but that is a little awkward to type. // Instances of this class are indexable, but that is a little awkward to type.
// The cells field gives typed access to that aspect of the instance. This is a // The cells field gives typed access to that aspect of the instance. This is a
// bit hacky, and should be cleaned up when BaseRowModel is ported to typescript. // bit hacky, and should be cleaned up when BaseRowModel is ported to typescript.
public readonly cells: {[key: string]: modelUtil.KoSaveableObservable<any>} = this as any; public readonly cells: {[key: string]: modelUtil.KoSaveableObservable<CellValue>} = this as any;
public _validationFailures: ko.PureComputed<Array<IRowModel<'_grist_Validations'>>>; public _validationFailures: ko.PureComputed<Array<IRowModel<'_grist_Validations'>>>;
public _isAddRow: ko.Observable<boolean>; public _isAddRow: ko.Observable<boolean>;

@ -1,6 +1,7 @@
import {DataRowModel} from 'app/client/models/DataRowModel'; import {DataRowModel} from 'app/client/models/DataRowModel';
import {colors} from 'app/client/ui2018/cssVars'; import {colors} from 'app/client/ui2018/cssVars';
import {ChoiceTextBox} from 'app/client/widgets/ChoiceTextBox'; import {ChoiceTextBox} from 'app/client/widgets/ChoiceTextBox';
import {CellValue} from 'app/common/DocActions';
import {decodeObject} from 'app/plugin/objtypes'; import {decodeObject} from 'app/plugin/objtypes';
import {Computed, dom, styled} from 'grainjs'; import {Computed, dom, styled} from 'grainjs';
@ -17,7 +18,9 @@ export class ChoiceListCell extends ChoiceTextBox {
dom.cls('field_clip'), dom.cls('field_clip'),
cssChoiceList.cls('-wrap', this.wrapping), cssChoiceList.cls('-wrap', this.wrapping),
dom.style('justify-content', this.alignment), dom.style('justify-content', this.alignment),
dom.domComputed((use) => use(row._isAddRow) ? null : [use(value), use(this._choiceSet)], (input) => { dom.domComputed((use) => {
return use(row._isAddRow) ? null : [use(value), use(this._choiceSet)] as [CellValue, Set<string>];
}, (input) => {
if (!input) { return null; } if (!input) { return null; }
const [rawValue, choiceSet] = input; const [rawValue, choiceSet] = input;
const val = decodeObject(rawValue); const val = decodeObject(rawValue);
@ -27,7 +30,7 @@ export class ChoiceListCell extends ChoiceTextBox {
return tokens.map(token => return tokens.map(token =>
cssToken( cssToken(
String(token), String(token),
cssInvalidToken.cls('-invalid', !choiceSet.has(token)) cssInvalidToken.cls('-invalid', !choiceSet.has(token as string))
) )
); );
}), }),

@ -1,3 +1,4 @@
import {CellValue} from 'app/common/DocActions';
import {DataRowModel} from 'app/client/models/DataRowModel'; import {DataRowModel} from 'app/client/models/DataRowModel';
import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec'; import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec';
import {colors, testId} from 'app/client/ui2018/cssVars'; import {colors, testId} from 'app/client/ui2018/cssVars';
@ -20,7 +21,7 @@ export class HyperLinkTextBox extends NTextBox {
return cssFieldClip( return cssFieldClip(
dom.style('text-align', this.alignment), dom.style('text-align', this.alignment),
dom.cls('text_wrapping', this.wrapping), dom.cls('text_wrapping', this.wrapping),
dom.maybe(value, () => dom.maybe((use) => Boolean(use(value)), () =>
dom('a', dom('a',
dom.attr('href', (use) => _constructUrl(use(value))), dom.attr('href', (use) => _constructUrl(use(value))),
dom.attr('target', '_blank'), dom.attr('target', '_blank'),
@ -40,7 +41,7 @@ export class HyperLinkTextBox extends NTextBox {
/** /**
* Formats value like `foo bar baz` by discarding `baz` and returning `foo bar`. * Formats value like `foo bar baz` by discarding `baz` and returning `foo bar`.
*/ */
function _formatValue(value: string|null|undefined): string { function _formatValue(value: CellValue): string {
if (typeof value !== 'string') { return ''; } if (typeof value !== 'string') { return ''; }
const index = value.lastIndexOf(' '); const index = value.lastIndexOf(' ');
return index >= 0 ? value.slice(0, index) : value; return index >= 0 ? value.slice(0, index) : value;
@ -50,7 +51,8 @@ function _formatValue(value: string|null|undefined): string {
* Given value like `foo bar baz`, constructs URL by checking if `baz` is a valid URL and, * Given value like `foo bar baz`, constructs URL by checking if `baz` is a valid URL and,
* if not, prepending `http://`. * if not, prepending `http://`.
*/ */
function _constructUrl(value: string): string { function _constructUrl(value: CellValue): string {
if (typeof value !== 'string') { return ''; }
const url = value.slice(value.lastIndexOf(' ') + 1); const url = value.slice(value.lastIndexOf(' ') + 1);
try { try {
// Try to construct a valid URL // Try to construct a valid URL

Loading…
Cancel
Save