From 9cd0bb74aaa45cb475e48f58b155438ba7c81506 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 28 Mar 2018 15:06:50 +0100 Subject: [PATCH 1/5] Change IDE diff view responsively Closes #44305 --- .../javascripts/ide/components/repo_editor.vue | 10 ++++++---- app/assets/javascripts/ide/lib/editor.js | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index b1a16350c19..383ec7740a6 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -13,7 +13,7 @@ export default { }, }, computed: { - ...mapState(['leftPanelCollapsed', 'rightPanelCollapsed', 'viewer', 'delayViewerUpdated']), + ...mapState(['rightPanelCollapsed', 'viewer', 'delayViewerUpdated', 'panelResizing']), ...mapGetters(['currentMergeRequest']), shouldHideEditor() { return this.file && this.file.binary && !this.file.raw; @@ -26,15 +26,17 @@ export default { this.initMonaco(); } }, - leftPanelCollapsed() { - this.editor.updateDimensions(); - }, rightPanelCollapsed() { this.editor.updateDimensions(); }, viewer() { this.createEditorInstance(); }, + panelResizing() { + if (!this.panelResizing) { + this.editor.updateDimensions(); + } + }, }, beforeDestroy() { this.editor.dispose(); diff --git a/app/assets/javascripts/ide/lib/editor.js b/app/assets/javascripts/ide/lib/editor.js index 6b4ba30e086..4f5106d1583 100644 --- a/app/assets/javascripts/ide/lib/editor.js +++ b/app/assets/javascripts/ide/lib/editor.js @@ -81,7 +81,7 @@ export default class Editor { } attachModel(model) { - if (this.instance.getEditorType() === 'vs.editor.IDiffEditor') { + if (this.isDiffEditorType) { this.instance.setModel({ original: model.getOriginalModel(), modified: model.getModel(), @@ -153,6 +153,7 @@ export default class Editor { updateDimensions() { this.instance.layout(); + this.updateDiffView(); } setPosition({ lineNumber, column }) { @@ -171,4 +172,16 @@ export default class Editor { this.disposable.add(this.instance.onDidChangeCursorPosition(e => cb(this.instance, e))); } + + updateDiffView() { + if (!this.isDiffEditorType) return; + + this.instance.updateOptions({ + renderSideBySide: this.instance.getDomNode().offsetWidth >= 700, + }); + } + + get isDiffEditorType() { + return this.instance.getEditorType() === 'vs.editor.IDiffEditor'; + } } From f1ddfac4b0062996b9a040c9e4ab1f23e87c345a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 28 Mar 2018 15:30:58 +0100 Subject: [PATCH 2/5] added specs --- .../ide/components/repo_editor_spec.js | 68 ++++++++++--------- spec/javascripts/ide/lib/editor_spec.js | 52 ++++++++++++++ 2 files changed, 87 insertions(+), 33 deletions(-) diff --git a/spec/javascripts/ide/components/repo_editor_spec.js b/spec/javascripts/ide/components/repo_editor_spec.js index 9d3fa1280f4..de31894c185 100644 --- a/spec/javascripts/ide/components/repo_editor_spec.js +++ b/spec/javascripts/ide/components/repo_editor_spec.js @@ -149,47 +149,49 @@ describe('RepoEditor', () => { }); }); - describe('setup editor for merge request viewing', () => { - beforeEach(done => { - // Resetting as the main test setup has already done it - vm.$destroy(); - resetStore(vm.$store); - Editor.editorInstance.modelManager.dispose(); + describe('editor updateDimensions', () => { + beforeEach(() => { + spyOn(vm.editor, 'updateDimensions').and.callThrough(); + spyOn(vm.editor, 'updateDiffView'); + }); - const f = { - ...file(), - active: true, - tempFile: true, - html: 'testing', - mrChange: { diff: 'ABC' }, - baseRaw: 'testing', - content: 'test', - }; - const RepoEditor = Vue.extend(repoEditor); - vm = createComponentWithStore(RepoEditor, store, { - file: f, - }); + it('calls updateDimensions when rightPanelCollapsed is changed', done => { + vm.$store.state.rightPanelCollapsed = true; - vm.$store.state.openFiles.push(f); - vm.$store.state.entries[f.path] = f; + vm.$nextTick(() => { + expect(vm.editor.updateDimensions).toHaveBeenCalled(); + expect(vm.editor.updateDiffView).toHaveBeenCalled(); - vm.$store.state.viewer = 'mrdiff'; - - vm.monaco = true; - - vm.$mount(); - - monacoLoader(['vs/editor/editor.main'], () => { - setTimeout(done, 0); + done(); }); }); - it('attaches merge request model to editor when merge request diff', () => { - spyOn(vm.editor, 'attachMergeRequestModel').and.callThrough(); + it('calls updateDimensions when panelResizing is false', done => { + vm.$store.state.panelResizing = true; - vm.setupEditor(); + vm + .$nextTick() + .then(() => { + vm.$store.state.panelResizing = false; + }) + .then(vm.$nextTick) + .then(() => { + expect(vm.editor.updateDimensions).toHaveBeenCalled(); + expect(vm.editor.updateDiffView).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); - expect(vm.editor.attachMergeRequestModel).toHaveBeenCalledWith(vm.model); + it('does not call updateDimensions when panelResizing is true', done => { + vm.$store.state.panelResizing = true; + + vm.$nextTick(() => { + expect(vm.editor.updateDimensions).not.toHaveBeenCalled(); + expect(vm.editor.updateDiffView).not.toHaveBeenCalled(); + + done(); + }); }); }); }); diff --git a/spec/javascripts/ide/lib/editor_spec.js b/spec/javascripts/ide/lib/editor_spec.js index ec56ebc0341..5aec4b5108c 100644 --- a/spec/javascripts/ide/lib/editor_spec.js +++ b/spec/javascripts/ide/lib/editor_spec.js @@ -215,4 +215,56 @@ describe('Multi-file editor library', () => { expect(instance.decorationsController.dispose).not.toHaveBeenCalled(); }); }); + + describe('updateDiffView', () => { + describe('edit mode', () => { + it('does not update options', () => { + instance.createInstance(holder); + + spyOn(instance.instance, 'updateOptions'); + + instance.updateDiffView(); + + expect(instance.instance.updateOptions).not.toHaveBeenCalled(); + }); + }); + + describe('diff mode', () => { + beforeEach(() => { + instance.createDiffInstance(holder); + + spyOn(instance.instance, 'updateOptions').and.callThrough(); + }); + + it('sets renderSideBySide to false if el is less than 700 pixels', () => { + spyOnProperty(instance.instance.getDomNode(), 'offsetWidth').and.returnValue(600); + + expect(instance.instance.updateOptions).not.toHaveBeenCalledWith({ + renderSideBySide: false, + }); + }); + + it('sets renderSideBySide to false if el is more than 700 pixels', () => { + spyOnProperty(instance.instance.getDomNode(), 'offsetWidth').and.returnValue(800); + + expect(instance.instance.updateOptions).not.toHaveBeenCalledWith({ + renderSideBySide: true, + }); + }); + }); + }); + + describe('isDiffEditorType', () => { + it('returns true when diff editor', () => { + instance.createDiffInstance(holder); + + expect(instance.isDiffEditorType).toBe(true); + }); + + it('returns false when not diff editor', () => { + instance.createInstance(holder); + + expect(instance.isDiffEditorType).toBe(false); + }); + }); }); From bc64e20cab52e5dfc1e703a09a6221134957f7ed Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Sun, 1 Apr 2018 16:47:10 +0100 Subject: [PATCH 3/5] updated wordWrap option be `on` --- app/assets/javascripts/ide/lib/editor_options.js | 2 +- spec/javascripts/ide/lib/editor_spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/ide/lib/editor_options.js b/app/assets/javascripts/ide/lib/editor_options.js index a213862f9b3..9f895d49f2e 100644 --- a/app/assets/javascripts/ide/lib/editor_options.js +++ b/app/assets/javascripts/ide/lib/editor_options.js @@ -6,7 +6,7 @@ export const defaultEditorOptions = { minimap: { enabled: false, }, - wordWrap: 'bounded', + wordWrap: 'on', }; export default [ diff --git a/spec/javascripts/ide/lib/editor_spec.js b/spec/javascripts/ide/lib/editor_spec.js index 5aec4b5108c..ce3997761b9 100644 --- a/spec/javascripts/ide/lib/editor_spec.js +++ b/spec/javascripts/ide/lib/editor_spec.js @@ -76,7 +76,7 @@ describe('Multi-file editor library', () => { occurrencesHighlight: false, renderLineHighlight: 'none', hideCursorInOverviewRuler: true, - wordWrap: 'bounded', + wordWrap: 'on', }); }); }); From 891164f10b49417fb0fbcb29638d346e9a56dd63 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 5 Apr 2018 10:24:18 +0100 Subject: [PATCH 4/5] set `renderSideBySide` when creating diff instance --- app/assets/javascripts/ide/lib/editor.js | 7 ++++++- spec/javascripts/ide/lib/editor_spec.js | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/ide/lib/editor.js b/app/assets/javascripts/ide/lib/editor.js index 4f5106d1583..0fa0ddecf13 100644 --- a/app/assets/javascripts/ide/lib/editor.js +++ b/app/assets/javascripts/ide/lib/editor.js @@ -69,6 +69,7 @@ export default class Editor { occurrencesHighlight: false, renderLineHighlight: 'none', hideCursorInOverviewRuler: true, + renderSideBySide: this.renderSideBySide(domElement), })), ); @@ -177,11 +178,15 @@ export default class Editor { if (!this.isDiffEditorType) return; this.instance.updateOptions({ - renderSideBySide: this.instance.getDomNode().offsetWidth >= 700, + renderSideBySide: this.renderSideBySide(this.instance.getDomNode()), }); } get isDiffEditorType() { return this.instance.getEditorType() === 'vs.editor.IDiffEditor'; } + + renderSideBySide(domElement) { + return domElement.offsetWidth >= 700; + } } diff --git a/spec/javascripts/ide/lib/editor_spec.js b/spec/javascripts/ide/lib/editor_spec.js index ce3997761b9..75e6f0f54ec 100644 --- a/spec/javascripts/ide/lib/editor_spec.js +++ b/spec/javascripts/ide/lib/editor_spec.js @@ -77,6 +77,7 @@ describe('Multi-file editor library', () => { renderLineHighlight: 'none', hideCursorInOverviewRuler: true, wordWrap: 'on', + renderSideBySide: true, }); }); }); From 8a043b6bed33f96d40c4cc37abebcbb327691a00 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 5 Apr 2018 10:46:19 +0100 Subject: [PATCH 5/5] fixed eslint --- app/assets/javascripts/ide/lib/editor.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/ide/lib/editor.js b/app/assets/javascripts/ide/lib/editor.js index 0fa0ddecf13..001737d6ee8 100644 --- a/app/assets/javascripts/ide/lib/editor.js +++ b/app/assets/javascripts/ide/lib/editor.js @@ -69,7 +69,7 @@ export default class Editor { occurrencesHighlight: false, renderLineHighlight: 'none', hideCursorInOverviewRuler: true, - renderSideBySide: this.renderSideBySide(domElement), + renderSideBySide: Editor.renderSideBySide(domElement), })), ); @@ -178,7 +178,7 @@ export default class Editor { if (!this.isDiffEditorType) return; this.instance.updateOptions({ - renderSideBySide: this.renderSideBySide(this.instance.getDomNode()), + renderSideBySide: Editor.renderSideBySide(this.instance.getDomNode()), }); } @@ -186,7 +186,7 @@ export default class Editor { return this.instance.getEditorType() === 'vs.editor.IDiffEditor'; } - renderSideBySide(domElement) { + static renderSideBySide(domElement) { return domElement.offsetWidth >= 700; } }