From 43719ecdd7b247ea2dd42c902588b26c8dfb8a9e Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Fri, 15 Sep 2023 16:09:40 +0200 Subject: [PATCH 1/5] feat: add FEEL editor for table cells Closes #774 --- .../css/dmn-js-decision-table-controls.css | 15 +++ .../cell-selection/CellSelectionUtil.js | 3 +- .../DecisionRulesCellEditorComponent.js | 110 +++++++++++------- .../src/features/keyboard/Keyboard.js | 4 +- .../test/helper/index.js | 14 +++ .../decision-rules/DecisionRulesEditorSpec.js | 74 ++++++++++-- .../src/components/EditableComponent.js | 2 +- .../src/components/LiteralExpression.js | 58 ++++++++- .../dmn-js-shared/test/util/EditorUtil.js | 2 +- 9 files changed, 215 insertions(+), 67 deletions(-) diff --git a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css index a68cd7716..a53a37e54 100644 --- a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css +++ b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css @@ -251,3 +251,18 @@ width: 20px; right: 0; } + +/* cell editor */ +.dmn-decision-table-container .cell-editor__placeholder { + position: absolute; +} + +.dmn-decision-table-container .cell-editor:focus-within .cell-editor__placeholder, +.dmn-decision-table-container .cell-editor:focus-within .dmn-expression-language { + display: none; +} + +.dmn-decision-table-container .cell-editor, +.dmn-decision-table-container .cell-editor .cm-scroller { + line-height: normal; +} diff --git a/packages/dmn-js-decision-table/src/features/cell-selection/CellSelectionUtil.js b/packages/dmn-js-decision-table/src/features/cell-selection/CellSelectionUtil.js index 92d406b44..ebfa83eb5 100644 --- a/packages/dmn-js-decision-table/src/features/cell-selection/CellSelectionUtil.js +++ b/packages/dmn-js-decision-table/src/features/cell-selection/CellSelectionUtil.js @@ -7,7 +7,6 @@ import { import cssEscape from 'css.escape'; import { - setRange, getRange } from 'selection-ranges'; @@ -109,6 +108,6 @@ export function ensureFocus(el) { const range = getRange(focusEl); if (!range || range.end === 0) { - setRange(focusEl, { start: 5000, end: 5000 }); + window.getSelection().setPosition(focusEl.firstChild, focusEl.firstChild.length); } } \ No newline at end of file diff --git a/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js b/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js index 794a6c3c1..3c4c153e7 100644 --- a/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js +++ b/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js @@ -4,7 +4,8 @@ import { isString } from 'min-dash'; import { is } from 'dmn-js-shared/lib/util/ModelUtil'; -import EditableComponent from 'dmn-js-shared/lib/components/EditableComponent'; +import ContentEditable from 'dmn-js-shared/lib/components/ContentEditable'; +import LiteralExpression from 'dmn-js-shared/lib/components/LiteralExpression'; import { Cell } from 'table-js/lib/components'; @@ -14,22 +15,14 @@ export default class DecisionRulesCellEditorComponent extends Component { constructor(props, context) { super(props, context); - this.state = { - isFocussed: false - }; - this.changeCellValue = this.changeCellValue.bind(this); - this.onFocus = this.onFocus.bind(this); - this.onBlur = this.onBlur.bind(this); this.onElementsChanged = this.onElementsChanged.bind(this); } - onElementsChanged() { this.forceUpdate(); } - componentWillMount() { const { injector } = this.context; @@ -42,35 +35,18 @@ export default class DecisionRulesCellEditorComponent extends Component { changeSupport.onElementsChanged(cell.id, this.onElementsChanged); } - componentWillUnmount() { const { cell } = this.props; this._changeSupport.offElementsChanged(cell.id, this.onElementsChanged); } - changeCellValue(value) { const { cell } = this.props; this._modeling.editCell(cell.businessObject, value); } - - onFocus() { - this.setState({ - isFocussed: true - }); - } - - - onBlur() { - this.setState({ - isFocussed: false - }); - } - - render() { const { cell, @@ -80,8 +56,6 @@ export default class DecisionRulesCellEditorComponent extends Component { colIndex } = this.props; - const { isFocussed } = this.state; - const isUnaryTest = is(cell, 'dmn:UnaryTests'); const businessObject = cell.businessObject; @@ -94,12 +68,7 @@ export default class DecisionRulesCellEditorComponent extends Component { data-col-id={ col.id } > @@ -108,8 +77,42 @@ export default class DecisionRulesCellEditorComponent extends Component { } } +class FeelEditor extends Component { + constructor(props, context) { + super(props, context); + this.state = { focussed: false }; -class TableCellEditor extends EditableComponent { + this.onFocus = this.onFocus.bind(this); + this.onBlur = this.onBlur.bind(this); + } + + onFocus() { + this.setState({ focussed: true }); + } + + onBlur() { + this.setState({ focussed: false }); + } + + render() { + if (this.state.focussed) { + return ; + } + + return
+ +
; + } +} + +class TableCellEditor extends Component { constructor(props, context) { super(props, context); @@ -164,10 +167,27 @@ class TableCellEditor extends EditableComponent { return this._expressionLanguages.getDefault(elementType); } + getEditor() { + return this.isFEEL() ? FeelEditor : ContentEditable; + } + + isFEEL() { + return this.getExpressionLanguage() === 'feel'; + } + + getExpressionLanguage() { + const { businessObject } = this.props; + + return businessObject.expressionLanguage || + this.getDefaultExpressionLanguage(businessObject).value; + } + render() { const { businessObject, - isFocussed + placeholder, + value, + onChange } = this.props; const description = this.getDescription(businessObject); @@ -178,21 +198,23 @@ class TableCellEditor extends EditableComponent { const isScript = this.isScript(businessObject); + const Editor = this.getEditor(); + return ( -
+
{ isString(description) - && !isFocussed &&
} + { - this.getEditor({ - className: isScript ? 'script-editor' : null - }) - } - { - !isDefaultExpressionLanguage && - !isFocussed && ( + !isDefaultExpressionLanguage && ( { + requestAnimationFrame(() => { + resolve(); + }); + }); +} diff --git a/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js b/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js index 2d9f772b6..44e1fafd8 100644 --- a/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js +++ b/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js @@ -1,4 +1,4 @@ -import { bootstrapModeler, inject } from 'test/helper'; +import { bootstrapModeler, inject, act } from 'test/helper'; import { query as domQuery } from 'min-dom'; @@ -8,7 +8,6 @@ import { queryEditor } from 'dmn-js-shared/test/util/EditorUtil'; import TestContainer from 'mocha-test-container-support'; -import simpleXML from '../../simple.dmn'; import emptyRuleXML from './empty-rule.dmn'; import languageExpressionXML from '../../expression-language.dmn'; @@ -49,18 +48,55 @@ describe('features/decision-rules', function() { describe('editing', function() { - beforeEach(bootstrapModeler(simpleXML, { + beforeEach(bootstrapModeler(languageExpressionXML, { modules: [ CoreModule, ModelingModule, DecisionRulesModule, DecisionRulesEditorModule ], - debounceInput: false + debounceInput: false, + expressionLanguages: { + options: CUSTOM_EXPRESSION_LANGUAGES + }, + })); + + + it('should edit cell (FEEL)', inject(async function(elementRegistry) { + + // given + const editor = queryEditor('[data-element-id="outputEntry2"]', testContainer); + + await act(() => editor.focus()); + + // when + await changeInput(document.activeElement, 'foo'); + + // then + expect(elementRegistry.get('outputEntry2').businessObject.text).to.equal('foo'); + })); + + + it('should edit cell - line breaks (FEEL)', inject(async function(elementRegistry) { + + // given + let editor = queryEditor('[data-element-id="outputEntry2"]', testContainer); + + await act(() => editor.focus()); + editor = document.activeElement; + + // when + await changeInput(editor, 'foo\nbar'); + + editor.blur(); + + // then + expect(elementRegistry.get('outputEntry2').businessObject.text) + .to.equal('foo\nbar'); })); - it('should edit cell', inject(function(elementRegistry) { + it('should edit cell (non-FEEL)', inject(function(elementRegistry) { // given const editor = queryEditor('[data-element-id="inputEntry1"]', testContainer); @@ -75,7 +111,7 @@ describe('features/decision-rules', function() { })); - it('should edit cell - line breaks', inject(function(elementRegistry) { + it('should edit cell - line breaks (non-FEEL)', inject(function(elementRegistry) { // given const editor = queryEditor('[data-element-id="inputEntry1"]', testContainer); @@ -146,7 +182,8 @@ describe('features/decision-rules', function() { editor.focus(); // then - expect(domQuery('.dmn-expression-language', cell)).to.not.exist; + const badge = domQuery('.dmn-expression-language', cell); + expect(badge).to.satisfy(isNotDisplayed); }); }); @@ -186,7 +223,8 @@ describe('features/decision-rules', function() { editor.focus(); // then - expect(domQuery('.dmn-expression-language', cell)).to.not.exist; + const badge = domQuery('.dmn-expression-language', cell); + expect(badge).to.satisfy(isNotDisplayed); }); }); @@ -244,7 +282,8 @@ describe('features/decision-rules', function() { editor.focus(); // then - expect(domQuery('.dmn-expression-language', cell)).to.not.exist; + const badge = domQuery('.dmn-expression-language', cell); + expect(badge).to.satisfy(isNotDisplayed); }); }); @@ -283,7 +322,8 @@ describe('features/decision-rules', function() { editor.focus(); // then - expect(domQuery('.dmn-expression-language', cell)).to.not.exist; + const badge = domQuery('.dmn-expression-language', cell); + expect(badge).to.satisfy(isNotDisplayed); }); }); @@ -329,8 +369,20 @@ describe('features/decision-rules', function() { }); - // helpers ////////////////// +function isNotDisplayed(element) { + return !element || getComputedStyle(element).display === 'none'; +} + +/** + * @param {HTMLElement} input + * @param {string} value + */ +function changeInput(input, value) { + return act(() => { + input.textContent = value; + }); +} function isFirefox() { return /Firefox/.test(window.navigator.userAgent); diff --git a/packages/dmn-js-shared/src/components/EditableComponent.js b/packages/dmn-js-shared/src/components/EditableComponent.js index 22457da47..3a97ecaa9 100644 --- a/packages/dmn-js-shared/src/components/EditableComponent.js +++ b/packages/dmn-js-shared/src/components/EditableComponent.js @@ -171,7 +171,7 @@ export default class EditableComponent extends Component { return ( { + handleMouseEvent = event => { event.stopPropagation(); }; - handleKeyDown = (event) => { + handleKeyDownCapture = event => { if (event.key === 'Enter') { + if (isAutocompleteOpen(this.node)) { + event.triggeredFromAutocomplete = true; + return; + } + + // supress non cmd+enter newline + if (this.props.ctrlForNewline && !isCmd(event)) { + event.preventDefault(); + } + + if (this.props.singleLine) { + event.preventDefault(); + } + } + }; + + /** + * @param {KeyboardEvent} event + */ + handleKeyDown = event => { + + // contain the event in the component to not trigger global handlers + if ([ 'Enter', 'Escape' ].includes(event.key) && event.triggeredFromAutocomplete) { event.stopPropagation(); - event.preventDefault(); } }; @@ -90,16 +125,27 @@ export default class LiteralExpression extends Component { } }; + setNode = node => { + this.node = node; + }; + render() { return (
this.node = node } + ref={ this.setNode } onClick={ this.handleMouseEvent } - onKeyDown={ this.handleKeyDown } onFocusIn={ this.props.onFocus } onFocusOut={ this.props.onBlur } /> ); } } + +function isCmd(event) { + return event.metaKey || event.ctrlKey; +} + +function isAutocompleteOpen(node) { + return node.querySelector('.cm-tooltip-autocomplete'); +} diff --git a/packages/dmn-js-shared/test/util/EditorUtil.js b/packages/dmn-js-shared/test/util/EditorUtil.js index 07de4322e..fa39c4d8d 100644 --- a/packages/dmn-js-shared/test/util/EditorUtil.js +++ b/packages/dmn-js-shared/test/util/EditorUtil.js @@ -1,5 +1,5 @@ import { query as domQuery } from 'min-dom'; export function queryEditor(baseSelector, container) { - return domQuery(baseSelector + ' .content-editable', container); + return domQuery(baseSelector + ' [contenteditable=true]', container); } \ No newline at end of file From b534b38c92fbc66ff668cec9a9bc0a4f5bb13882 Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Fri, 22 Sep 2023 09:14:21 +0200 Subject: [PATCH 2/5] fix: do not close input editor on enter --- .../editor/components/InputEditor.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/dmn-js-decision-table/src/features/decision-table-head/editor/components/InputEditor.js b/packages/dmn-js-decision-table/src/features/decision-table-head/editor/components/InputEditor.js index ef337bb3a..67f32a856 100644 --- a/packages/dmn-js-decision-table/src/features/decision-table-head/editor/components/InputEditor.js +++ b/packages/dmn-js-decision-table/src/features/decision-table-head/editor/components/InputEditor.js @@ -44,6 +44,16 @@ export default class InputEditor extends Component { return LiteralExpression; } + /** + * Supress default menu closure on enter. + * @param {KeyboardEvent} event + */ + handleKeyDown = event => { + if (event.key === 'Enter') { + event.stopPropagation(); + } + }; + render() { const { @@ -54,7 +64,8 @@ export default class InputEditor extends Component { const ExpressionEditor = this.getExpressionEditorComponent(); return ( -
+
Date: Fri, 22 Sep 2023 14:00:13 +0200 Subject: [PATCH 3/5] fix: solve the replaceChild problem This solves a problem of `replaceChild` argument not being a child of supposed parent. Related to changes in VDOM in reaction to focus. --- .../css/dmn-js-decision-table-controls.css | 4 +++ .../DecisionRulesCellEditorComponent.js | 25 +++++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css index a53a37e54..8b81b3b89 100644 --- a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css +++ b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css @@ -266,3 +266,7 @@ .dmn-decision-table-container .cell-editor .cm-scroller { line-height: normal; } + +.dmn-decision-table-container .cell-editor .feel-editor.focussed > :nth-child(2) { + display: none; +} diff --git a/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js b/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js index 3c4c153e7..8ec21ee5d 100644 --- a/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js +++ b/packages/dmn-js-decision-table/src/features/decision-rules/components/DecisionRulesCellEditorComponent.js @@ -95,17 +95,22 @@ class FeelEditor extends Component { } render() { - if (this.state.focussed) { - return ; - } - - return
+ const { focussed } = this.state; + const className = `feel-editor${focussed ? ' focussed' : ''}`; + + // TODO(@barmac): display only a single editor; + // required to workaround "replaceChild" error + return
+ { focussed && + + } {} } onFocus={ this.onFocus } />
; @@ -207,7 +212,7 @@ class TableCellEditor extends Component { &&
} Date: Fri, 22 Sep 2023 14:45:11 +0200 Subject: [PATCH 4/5] test: adjust tests --- .../spec/features/decision-rules/DecisionRulesEditorSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js b/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js index 44e1fafd8..c0e73a566 100644 --- a/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js +++ b/packages/dmn-js-decision-table/test/spec/features/decision-rules/DecisionRulesEditorSpec.js @@ -352,7 +352,7 @@ describe('features/decision-rules', function() { const editor = queryEditor('[data-element-id="unaryTest_1"]', testContainer); // then - expect(editor.textContent).to.eql('-'); + expect(editor.matches('[data-placeholder="-"]')).to.be.true; })); @@ -362,7 +362,7 @@ describe('features/decision-rules', function() { const editor = queryEditor('[data-element-id="outputEntry_1"]', testContainer); // then - expect(editor.textContent).to.eql(''); + expect(editor.matches('[data-placeholder="-"]')).to.be.false; })); }); From 5a2b420a0371db9d3cb7a56aefc90360fb7a709a Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Mon, 25 Sep 2023 09:28:59 +0200 Subject: [PATCH 5/5] feat: use monospace for cell editor --- .../assets/css/dmn-js-decision-table-controls.css | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css index 8b81b3b89..d5260fbca 100644 --- a/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css +++ b/packages/dmn-js-decision-table/assets/css/dmn-js-decision-table-controls.css @@ -264,7 +264,8 @@ .dmn-decision-table-container .cell-editor, .dmn-decision-table-container .cell-editor .cm-scroller { - line-height: normal; + line-height: 1; + font-family: monospace; } .dmn-decision-table-container .cell-editor .feel-editor.focussed > :nth-child(2) {