From 4ee7ea1e4b97b429a4ec9fc14d4861a114dcae1b Mon Sep 17 00:00:00 2001 From: Beatriz Mendes Date: Mon, 16 Jan 2023 10:58:15 +0100 Subject: [PATCH 1/4] feat(popup-menu): allow entries to be draggable --- lib/features/popup-menu/PopupMenu.js | 17 +++- lib/features/popup-menu/PopupMenuComponent.js | 2 +- lib/features/popup-menu/PopupMenuItem.js | 10 ++- lib/features/popup-menu/PopupMenuList.js | 8 +- .../popup-menu/PopupMenuComponentSpec.js | 28 ++++++- .../spec/features/popup-menu/PopupMenuSpec.js | 81 +++++++++++++++++++ 6 files changed, 133 insertions(+), 13 deletions(-) diff --git a/lib/features/popup-menu/PopupMenu.js b/lib/features/popup-menu/PopupMenu.js index bf6bd29d7..f3286473f 100644 --- a/lib/features/popup-menu/PopupMenu.js +++ b/lib/features/popup-menu/PopupMenu.js @@ -110,7 +110,7 @@ PopupMenu.prototype._render = function() { const scale = this._updateScale(this._current.container); const onClose = result => this.close(result); - const onSelect = (event, entry) => this.trigger(event, entry); + const onSelect = (event, entry, action) => this.trigger(event, entry, action); render( html` @@ -507,10 +507,11 @@ PopupMenu.prototype.isOpen = function() { * * @param {Object} event * @param {Object} entry + * @param {string} [action='click'] the action to trigger * * @return the result of the action callback, if any */ -PopupMenu.prototype.trigger = function(event, entry) { +PopupMenu.prototype.trigger = function(event, entry, action = 'click') { // silence other actions event.preventDefault(); @@ -522,8 +523,16 @@ PopupMenu.prototype.trigger = function(event, entry) { entry = this._getEntry(entryId); } - if (entry.action) { - return entry.action.call(null, event, entry); + const handler = entry.action; + + if (isFunction(handler)) { + if (action === 'click') { + return handler(event, entry); + } + } else { + if (handler[action]) { + return handler[action](event, entry); + } } }; diff --git a/lib/features/popup-menu/PopupMenuComponent.js b/lib/features/popup-menu/PopupMenuComponent.js index 05db484f5..17661a252 100644 --- a/lib/features/popup-menu/PopupMenuComponent.js +++ b/lib/features/popup-menu/PopupMenuComponent.js @@ -232,7 +232,7 @@ export default function PopupMenuComponent(props) { entries=${ entries } selectedEntry=${ selectedEntry } setSelectedEntry=${ setSelectedEntry } - onClick=${ onSelect } + onAction=${ onSelect } /> ${ entries.length === 0 && html` diff --git a/lib/features/popup-menu/PopupMenuItem.js b/lib/features/popup-menu/PopupMenuItem.js index 3504d3a64..b5f75b859 100644 --- a/lib/features/popup-menu/PopupMenuItem.js +++ b/lib/features/popup-menu/PopupMenuItem.js @@ -13,7 +13,7 @@ import { * @param {boolean} selected * @param {function} onMouseEnter * @param {function} onMouseLeave - * @param {function} onClick + * @param {function} onAction */ export default function PopupMenuItem(props) { const { @@ -21,17 +21,21 @@ export default function PopupMenuItem(props) { selected, onMouseEnter, onMouseLeave, - onClick + onAction } = props; + const draggable = entry.action && entry.action.dragstart; + return html`
  • onAction(event, entry, 'dragstart') } + draggable=${ draggable } >
    setSelectedEntry(entry) } onMouseLeave=${ () => setSelectedEntry(null) } - onClick=${ onClick } + ...${ restProps } /> `) } diff --git a/test/spec/features/popup-menu/PopupMenuComponentSpec.js b/test/spec/features/popup-menu/PopupMenuComponentSpec.js index f9534d481..71e245b02 100644 --- a/test/spec/features/popup-menu/PopupMenuComponentSpec.js +++ b/test/spec/features/popup-menu/PopupMenuComponentSpec.js @@ -222,7 +222,8 @@ describe('features/popup-menu - ', function() { { id: '16', group: { id: 'other', name: 'Other' }, label: '16' }, { id: '17', group: { id: 'other', name: 'Other' }, label: '17' }, { id: '18', group: { id: 'other', name: 'Other' }, label: '18' }, - { id: '19', group: { id: 'other', name: 'Other' }, label: '19' } + { id: '19', group: { id: 'other', name: 'Other' }, label: '19' }, + { id: '20', label: 'Draggable', action: { dragstart: () => {} } }, ]; createPopupMenu({ @@ -391,6 +392,23 @@ describe('features/popup-menu - ', function() { expect(groupHeader.textContent).to.eql('SOME GROUP'); }); + + it('should allow to drag entries', function() { + + // given + const entries = [ { id: '1', label: '1', action: { dragstart: ()=> {} } } ]; + + const onSelectSpy = sinon.spy(); + + createPopupMenu({ container, entries, onSelect: onSelectSpy }); + + // when + domQuery('.entry', container).dispatchEvent(dragStart()); + + // then + expect(onSelectSpy).to.have.been.calledOnce; + }); + }); @@ -811,4 +829,12 @@ function keyDown(key) { */ function keyUp(key) { return new KeyboardEvent('keyup', { key, bubbles: true }); +} + +/** + * + * @return { DragEvent } + */ +function dragStart() { + return new DragEvent('dragstart'); } \ No newline at end of file diff --git a/test/spec/features/popup-menu/PopupMenuSpec.js b/test/spec/features/popup-menu/PopupMenuSpec.js index 5828943f0..a16606c40 100755 --- a/test/spec/features/popup-menu/PopupMenuSpec.js +++ b/test/spec/features/popup-menu/PopupMenuSpec.js @@ -707,6 +707,87 @@ describe('features/popup-menu', function() { expect(triggerHeaderEntry).to.eql('Entry 3'); })); + + describe('complex handler', function() { + + it('should trigger click', inject(function(popupMenu) { + + // given + popupMenu.registerProvider('test-menu', { + getEntries: function() { + return [ + { + id: '1', + label: 'Entry 1', + className: 'Entry_1', + action: { + click: () => 'Entry 1 click', + dragstart: () => 'Entry 1 dragstart' + } + } + ]; + }, + getHeaderEntries: function() { + return [ + { + id: '2', + label: 'Entry 2', + className: 'Entry_2', + action: { + click: () => 'Entry 2 click' + } + } + ]; + } + }); + + popupMenu.open({}, 'test-menu', { x: 100, y: 100 }); + + var entry = queryEntry('1'); + var headerEntry = queryEntry('2'); + + // when + var triggerEntry = popupMenu.trigger(globalEvent(entry, { x: 0, y: 0 })); + var triggerHeaderEntry = popupMenu.trigger(globalEvent(headerEntry, { x: 0, y: 0 })); + + // then + expect(triggerEntry).to.eql('Entry 1 click'); + expect(triggerHeaderEntry).to.eql('Entry 2 click'); + })); + + + it('should trigger dragstart', inject(function(popupMenu) { + + // given + popupMenu.registerProvider('test-menu', { + getEntries: function() { + return [ + { + id: '1', + label: 'Entry 1', + className: 'Entry_1', + action: { + click: () => 'Entry 1 click', + dragstart: () => 'Entry 1 dragstart' + } + } + ]; + } + }); + + popupMenu.open({}, 'test-menu', { x: 100, y: 100 }); + + var entry = queryEntry('1'); + + // when + var triggerEntry = popupMenu.trigger(globalEvent(entry, { x: 0, y: 0 }), null, 'dragstart'); + + // then + expect(triggerEntry).to.eql('Entry 1 dragstart'); + })); + + }); + }); From c13c0f822f0c175d95ed9302a0c0f33c03b27911 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 16 Jan 2023 15:27:07 +0100 Subject: [PATCH 2/4] test(popup-menu): make complex an actual integration test --- .../popup-menu/PopupMenuComponentSpec.js | 58 ---------- .../spec/features/popup-menu/PopupMenuSpec.js | 106 ++++++++++++++++++ 2 files changed, 106 insertions(+), 58 deletions(-) diff --git a/test/spec/features/popup-menu/PopupMenuComponentSpec.js b/test/spec/features/popup-menu/PopupMenuComponentSpec.js index 71e245b02..a4355ce04 100644 --- a/test/spec/features/popup-menu/PopupMenuComponentSpec.js +++ b/test/spec/features/popup-menu/PopupMenuComponentSpec.js @@ -182,64 +182,6 @@ describe('features/popup-menu - ', function() { }); - it('should render complex', function() { - - const imageUrl = TEST_IMAGE_URL; - - // given - const headerEntries = [ - { id: '1', label: '|A|' }, - { id: '1.1', label: '|A|', imageUrl }, - { id: '2', imageUrl, title: 'Toggle foo' }, - { id: '3', className: 'bpmn-icon-sun' } - ]; - - const iconGroup = { - id: 'icons', - name: 'Icon Group' - }; - - const entries = [ - { id: '4', label: 'Just label' }, - { id: '5', imageUrl, title: 'Toggle foo' }, - { id: '6', imageUrl, label: 'Toggle foo' }, - { id: '7', label: 'with description', description: 'I DESCRIBE IT' }, - { id: '7.1', label: 'with long title and description, you cannot believe what happened next', description: 'A very long description, you cannot believe what happened next' }, - { id: '7.2', label: 'with long title and description, you cannot believe what happened next', description: 'A very long description, you cannot believe what happened next', documentationRef: 'http://localhost' }, - { id: '8', imageUrl, label: 'with image + description', description: 'I DESCRIBE more stuff' }, - { id: '9', imageUrl, label: 'WITH DOC REF', documentationRef: 'http://localhost' }, - { id: '10', imageUrl, label: 'FOO', description: 'WITH DOC REF', documentationRef: 'http://localhost' }, - { id: '11', className: 'bpmn-icon-sun', label: 'FONT ICON + description', description: 'WITH DOC REF', group: iconGroup }, - { id: '11.1', className: 'bpmn-icon-sun', label: 'FONT ICON', group: iconGroup }, - { id: '11.2', className: 'bpmn-icon-sun', title: 'icon only', group: iconGroup }, - { id: '12', className: 'bpmn-icon-sun', title: 'icon only', group: { - id: 'super long', - name: 'Extremely super long group incredible!' - } }, - { id: '13', group: { id: 'other', name: 'Other' }, label: '13' }, - { id: '14', group: { id: 'other', name: 'Other' }, label: '14' }, - { id: '15', group: { id: 'other', name: 'Other' }, label: '15' }, - { id: '16', group: { id: 'other', name: 'Other' }, label: '16' }, - { id: '17', group: { id: 'other', name: 'Other' }, label: '17' }, - { id: '18', group: { id: 'other', name: 'Other' }, label: '18' }, - { id: '19', group: { id: 'other', name: 'Other' }, label: '19' }, - { id: '20', label: 'Draggable', action: { dragstart: () => {} } }, - ]; - - createPopupMenu({ - container, - title: 'Popup menu with super long title', - headerEntries, - entries, - position: () => { - return { x: 100, y: 200 }; - }, - width: 250 - }); - - }); - - describe('close', function() { it('should close on background click', function() { diff --git a/test/spec/features/popup-menu/PopupMenuSpec.js b/test/spec/features/popup-menu/PopupMenuSpec.js index a16606c40..71c14bd39 100755 --- a/test/spec/features/popup-menu/PopupMenuSpec.js +++ b/test/spec/features/popup-menu/PopupMenuSpec.js @@ -1848,6 +1848,112 @@ describe('features/popup-menu', function() { }); +describe('features/popup-menu - integration', function() { + + const imageUrl = `data:image/svg+xml;utf8,${ + encodeURIComponent(` + + + + `) + }`; + + const entrySet = (entries) => { + + return entries.reduce((keyedEntries, entry) => { + + expect(entry).to.have.property('id'); + expect(keyedEntries).not.to.have.property(entry.id); + + keyedEntries[entry.id] = entry; + + return keyedEntries; + }, {}); + + }; + + beforeEach(bootstrapDiagram({ + modules: [ + popupMenuModule + ] + })); + + + it('should render complex', inject(function(popupMenu) { + + // given + const headerEntries = entrySet([ + { id: '_1', label: '|A|' }, + { id: '_1.1', label: '|A|', imageUrl }, + { id: '_2', imageUrl, title: 'Toggle foo' }, + { id: '_3', className: 'bpmn-icon-sun' } + ]); + + const iconGroup = { + id: '_icons', + name: 'Icon Group' + }; + + const entries = entrySet([ + { id: '_4', label: 'Label (click)', action: { + click(...args) { + console.log('CLICK', ...args); + } + } }, + { id: '_4.1', label: 'Label (drag)', action: { + dragstart(...args) { + console.log('DRAG START', ...args); + } + } }, + { id: '_4.2', label: 'Label (click + drag)', action: { + dragstart(...args) { + console.log('DRAG START', ...args); + }, + click(...args) { + console.log('CLICK', ...args); + } + } }, + { id: '_5', imageUrl, title: 'Just image' }, + { id: '_6', imageUrl, label: 'Image and label' }, + { id: '_7', label: 'with description', description: 'I DESCRIBE IT' }, + { id: '_7.1', label: 'with long title and description, you cannot believe what happened next', description: 'A very long description, you cannot believe what happened next' }, + { id: '_7.2', label: 'with long title and description, you cannot believe what happened next', description: 'A very long description, you cannot believe what happened next', documentationRef: 'http://localhost' }, + { id: '_8', imageUrl, label: 'with image + description', description: 'I DESCRIBE more stuff' }, + { id: '_9', imageUrl, label: 'WITH DOC REF', documentationRef: 'http://localhost' }, + { id: '_10', imageUrl, label: 'FOO', description: 'WITH DOC REF', documentationRef: 'http://localhost' }, + { id: '_11', className: 'bpmn-icon-sun', label: 'FONT ICON + description', description: 'WITH DOC REF', group: iconGroup }, + { id: '_11.1', className: 'bpmn-icon-sun', label: 'FONT ICON', group: iconGroup }, + { id: '_11.2', className: 'bpmn-icon-sun', title: 'icon only', group: iconGroup }, + { id: '_12', className: 'bpmn-icon-sun', title: 'icon only', group: { + id: '_super long', + name: 'Extremely super long group incredible!' + } }, + { id: '_13', group: { id: '_other', name: 'Other' }, label: '13' }, + { id: '_14', group: { id: '_other', name: 'Other' }, label: '14' }, + { id: '_15', group: { id: '_other', name: 'Other' }, label: '15' }, + { id: '_16', group: { id: '_other', name: 'Other' }, label: '16' }, + { id: '_17', group: { id: '_other', name: 'Other' }, label: '17' }, + { id: '_18', group: { id: '_other', name: 'Other' }, label: '18' }, + { id: '_19', group: { id: '_other', name: 'Other' }, label: '19' } + ]); + + const provider = new Provider(entries, headerEntries); + + // when + popupMenu.registerProvider('complex', provider); + + // then + popupMenu.open({}, 'complex', { + x: 100, y: 200 + }, { + width: 250, + title: 'Popup menu with super long title' + }); + + })); + +}); + // helpers ///////////// From 988373649746ac4708795d7c78687d84d618e1e8 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 16 Jan 2023 19:55:20 +0100 Subject: [PATCH 3/4] chore(popup-menu): simplify entries generation --- lib/features/popup-menu/PopupMenu.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/features/popup-menu/PopupMenu.js b/lib/features/popup-menu/PopupMenu.js index f3286473f..f2cabaf95 100644 --- a/lib/features/popup-menu/PopupMenu.js +++ b/lib/features/popup-menu/PopupMenu.js @@ -91,17 +91,13 @@ PopupMenu.prototype._render = function() { options } = this._current; - const entriesArray = [ - ...Object.entries(entries).map( - ([ key, value ]) => ({ id: key, ...value }) - ) - ]; + const entriesArray = Object.entries(entries).map( + ([ key, value ]) => ({ id: key, ...value }) + ); - const headerEntriesArray = [ - ...Object.entries(headerEntries).map( - ([ key, value ]) => ({ id: key, ...value }) - ) - ]; + const headerEntriesArray = Object.entries(headerEntries).map( + ([ key, value ]) => ({ id: key, ...value }) + ); const position = _position && ( (container) => this._ensureVisible(container, _position) From 805f203019424472827b36fc672af23169a0b72d Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 16 Jan 2023 20:15:09 +0100 Subject: [PATCH 4/4] chore(popup-menu): make always draggable Prevents ugly text selection effect / artifact if entry does not feature a drag action. --- lib/features/popup-menu/PopupMenuItem.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/features/popup-menu/PopupMenuItem.js b/lib/features/popup-menu/PopupMenuItem.js index b5f75b859..5927bd4ce 100644 --- a/lib/features/popup-menu/PopupMenuItem.js +++ b/lib/features/popup-menu/PopupMenuItem.js @@ -24,8 +24,6 @@ export default function PopupMenuItem(props) { onAction } = props; - const draggable = entry.action && entry.action.dragstart; - return html`
  • onAction(event, entry, 'dragstart') } - draggable=${ draggable } + draggable=${ true } >