From 964f209c404e5aa6e62dbbd7c0f70a92605b318f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 15:33:20 +0200 Subject: [PATCH] Don't throw an error when setting an incredibly small `lineHeight` Instead, if the measured line height equals 0, default it to 1 so that the editor component doesn't start computing `NaN` or `Infinity` values due to e.g. dividing by 0. We should probably consider sanitizing line heights smaller than a certain threshold, but that's non trivial because line height is expressed as a multiplier of the font size. Also, users may style the `line-height` property via CSS, which may still throw errors when using small values. --- spec/text-editor-component-spec.js | 38 ++++++++++++++++++++++++++++++ src/text-editor-component.js | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 2c13b48af..b2f94b097 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -119,6 +119,44 @@ describe('TextEditorComponent', () => { } }) + it('re-renders lines when their height changes', async () => { + const {component, element, editor} = buildComponent({rowsPerTile: 3, autoHeight: false}) + element.style.height = 4 * component.measurements.lineHeight + 'px' + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(9) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(9) + + element.style.lineHeight = '2.0' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(6) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(6) + + element.style.lineHeight = '0.7' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(12) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(12) + + element.style.lineHeight = '0.05' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(13) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(13) + + element.style.lineHeight = '0' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(13) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(13) + + element.style.lineHeight = '1' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(9) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(9) + }) + it('makes the content at least as tall as the scroll container client height', async () => { const {component, element, editor} = buildComponent({text: 'a', height: 100}) expect(component.refs.content.offsetHeight).toBe(100) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 970ee29c1..f4401ad8c 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -2046,7 +2046,7 @@ class TextEditorComponent { } measureCharacterDimensions () { - this.measurements.lineHeight = this.refs.characterMeasurementLine.getBoundingClientRect().height + this.measurements.lineHeight = Math.max(1, this.refs.characterMeasurementLine.getBoundingClientRect().height) this.measurements.baseCharacterWidth = this.refs.normalWidthCharacterSpan.getBoundingClientRect().width this.measurements.doubleWidthCharacterWidth = this.refs.doubleWidthCharacterSpan.getBoundingClientRect().width this.measurements.halfWidthCharacterWidth = this.refs.halfWidthCharacterSpan.getBoundingClientRect().width