From 4e805a4d9cea92e6a32ba6b6c108f9cca34a4abe Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 20 Jul 2022 19:40:22 -0400 Subject: [PATCH] (core) Fix sizing of tracebacks in formula errors, to make it scrollable Summary: When traceback is present, give it 64px, or more if available, or less if less is needed. If less space is available than needed, the traceback will scroll within its allocated area. Test Plan: The test FieldEditorSizing which tests basic sizing still passes; details with different size of formula and traceback were tested manually. Reviewers: jarek Reviewed By: jarek Subscribers: jarek Differential Revision: https://phab.getgrist.com/D3531 --- app/client/components/AceEditor.js | 8 +++++- app/client/widgets/EditorPlacement.ts | 1 + app/client/widgets/FormulaEditor.ts | 38 ++++++++++++++++++++------- app/client/widgets/TextEditor.css | 27 +++++++++++++------ 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/app/client/components/AceEditor.js b/app/client/components/AceEditor.js index 80ca1030..c375729e 100644 --- a/app/client/components/AceEditor.js +++ b/app/client/components/AceEditor.js @@ -234,11 +234,17 @@ AceEditor.prototype._setup = function() { AceEditor.prototype.resize = function() { var wrap = this.session.getUseWrapMode(); var contentWidth = wrap ? 0 : this._getContentWidth(); + var contentHeight = this._getContentHeight(); var desiredSize = { width: wrap ? 0 : contentWidth + this.textPadding, - height: this._getContentHeight() + height: contentHeight, }; var size = this.calcSize(this._fullDom, desiredSize); + if (size.height < contentHeight) { + // Editor will show a vertical scrollbar, so recalculate to make space for it. + desiredSize.width += 20; + size = this.calcSize(this._fullDom, desiredSize); + } if (size.width < contentWidth) { // Editor will show a horizontal scrollbar, so recalculate to make space for it. desiredSize.height += 20; diff --git a/app/client/widgets/EditorPlacement.ts b/app/client/widgets/EditorPlacement.ts index b76c569e..673141d0 100644 --- a/app/client/widgets/EditorPlacement.ts +++ b/app/client/widgets/EditorPlacement.ts @@ -115,6 +115,7 @@ export class EditorPlacement extends Disposable { // stretch to the calculated width (so need an explicit value), but may be dynamic in // height. (This feels hacky, but solves the problem.) width: width + 'px', + 'max-height': maxHeight + 'px', }); } diff --git a/app/client/widgets/FormulaEditor.ts b/app/client/widgets/FormulaEditor.ts index d65e3532..83fa5b28 100644 --- a/app/client/widgets/FormulaEditor.ts +++ b/app/client/widgets/FormulaEditor.ts @@ -93,8 +93,12 @@ export class FormulaEditor extends NewBaseEditor { return error.details ?? ""; }); + // Once the exception details are available, update the sizing. The extra delay is to allow + // the DOM to update before resizing. + this.autoDispose(errorDetails.addListener(() => setTimeout(() => this._formulaEditor.resize(), 0))); + this.autoDispose(this._formulaEditor); - this._dom = dom('div.default_editor', + this._dom = dom('div.default_editor.formula_editor_wrapper', // switch border shadow dom.cls("readonly_editor", options.readonly), createMobileButtons(options.commands), @@ -128,8 +132,7 @@ export class FormulaEditor extends NewBaseEditor { aceObj.once("change", () => editingFormula(true)); }) ), - (options.formulaError ? - dom('div.error_box', + (options.formulaError ? [ dom('div.error_msg', testId('formula-error-msg'), dom.on('click', () => { if (errorDetails.get()){ @@ -142,14 +145,15 @@ export class FormulaEditor extends NewBaseEditor { ), dom.text(errorText), ), - dom.maybe(errorDetails, () => + dom.maybe(use => Boolean(use(errorDetails) && !use(hideErrDetails)), () => dom('div.error_details', - dom.hide(hideErrDetails), - dom.text(errorDetails), + dom('div.error_details_inner', + dom.text(errorDetails), + ), testId('formula-error-details'), ) ) - ) : null + ] : null ) ); } @@ -181,12 +185,28 @@ export class FormulaEditor extends NewBaseEditor { } private _calcSize(elem: HTMLElement, desiredElemSize: ISize) { + const errorBox: HTMLElement|null = this._dom.querySelector('.error_details'); + const errorBoxStartHeight = errorBox?.getBoundingClientRect().height || 0; + const errorBoxDesiredHeight = errorBox?.scrollHeight || 0; + // If we have an error to show, ask for a larger size for formulaEditor. const desiredSize = { width: Math.max(desiredElemSize.width, (this.options.formulaError ? minFormulaErrorWidth : 0)), - height: desiredElemSize.height, + // Ask for extra space for the error; we'll decide how to allocate it below. + height: desiredElemSize.height + (errorBoxDesiredHeight - errorBoxStartHeight), }; - return this._editorPlacement.calcSizeWithPadding(elem, desiredSize); + const result = this._editorPlacement.calcSizeWithPadding(elem, desiredSize); + if (errorBox) { + // Note that result.height does not include errorBoxStartHeight, but includes any available + // extra space that we requested. + const availableForError = errorBoxStartHeight + (result.height - desiredElemSize.height); + // This is the key calculation: if space is available, use it; if not, give 64px to error + // (it'll scroll within that), but don't use more than desired. + const errorBoxEndHeight = Math.min(errorBoxDesiredHeight, Math.max(availableForError, 64)); + errorBox.style.height = `${errorBoxEndHeight}px`; + result.height -= (errorBoxEndHeight - errorBoxStartHeight); + } + return result; } // TODO: update regexes to unicode? diff --git a/app/client/widgets/TextEditor.css b/app/client/widgets/TextEditor.css index 086a66af..4faae513 100644 --- a/app/client/widgets/TextEditor.css +++ b/app/client/widgets/TextEditor.css @@ -1,6 +1,7 @@ .cell_editor { position: absolute; z-index: 1000; /* make it higher than popper's 999 */ + display: flex; } .default_editor { @@ -32,6 +33,11 @@ -webkit-mask-image: var(--icon-Lock); } +.formula_editor_wrapper { + display: flex; + flex-direction: column; + width: 100%; +} /* Make overflow hidden, since editor might be 1 pixel bigger due to fix for devices * with different pixel ratio */ @@ -40,6 +46,8 @@ padding: 4px 0 2px 21px; z-index: 10; overflow: hidden; + flex: none; + min-height: 22px; /* this is the usual height, but helps slightly when font is shorter than expected */ } /* styles specific to the formula editor in the side panel */ @@ -100,23 +108,26 @@ } .error_msg { + background-color: #ffb6c1; + padding: 4px; color: black; cursor: pointer; - padding: 4px; + white-space: pre-wrap; + flex: none; } .error_details { - padding: 2px 2px 2px 2px; - background-color: #F8ECEA; - margin: 0 0 -2px 0; + background-color: #F8EAD5; /* 20% of color-warning-bg */ font-family: 'Monaco', 'Menlo', monospace; font-size: 12px; + white-space: pre-wrap; + flex: auto; + overflow: auto; + word-break: break-all; } -.error_box { - background-color: #ffb6c1; - padding: 2px 0px 2px 0px; - white-space: pre-wrap; +.error_details_inner { + padding: 2px 2px 2px 28px; } .kf_collapser {