From 0f7708f42f39ac33ab62c9bad448bf5e59dc8203 Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Tue, 22 Oct 2019 15:36:32 +0200 Subject: [PATCH 1/2] fix(context-menu): focus the context menu after the position is set This fixes the jump of the table when the context menu was opened in a scrolled table. Previously, the focus was set on a context menu positioned in the top left corner of the table which forced the browser to scroll the position to the top left corner. Then, the position of the context menu was correctly set, but the scroll position was already changed. Now, the context menu is positioned correctly first and only then can it be focused. Related to https://github.com/camunda/camunda-modeler/issues/1433 --- .../components/ContextMenuComponent.js | 3 +- .../components/ContextMenuComponentSpec.js | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/features/context-menu/components/ContextMenuComponent.js b/src/features/context-menu/components/ContextMenuComponent.js index 760ca36..f1c89a2 100644 --- a/src/features/context-menu/components/ContextMenuComponent.js +++ b/src/features/context-menu/components/ContextMenuComponent.js @@ -227,12 +227,11 @@ class ContextMenu extends Component { } = this.props; if (node) { + this.updatePosition(); if (autoFocus) { ensureFocus(node); } - - this.updatePosition(); } } diff --git a/test/spec/features/context-menu/components/ContextMenuComponentSpec.js b/test/spec/features/context-menu/components/ContextMenuComponentSpec.js index ec7bc76..d9eaaaf 100644 --- a/test/spec/features/context-menu/components/ContextMenuComponentSpec.js +++ b/test/spec/features/context-menu/components/ContextMenuComponentSpec.js @@ -341,6 +341,53 @@ describe('features/context-menu - ContextMenuComponent', function() { )); + it('should set position before the context menu is focused', function(done) { + const test = inject( + function(components, contextMenu, eventBus, injector) { + + // given + const WithContext = withContext(ContextMenuComponent, { + injector, + eventBus + }); + + const renderedTree = renderIntoDocument(); + + + components.onGetComponent( + 'context-menu', + () => () => + ); + + // when + contextMenu.open({ + x: 100, + y: 100 + }, { autoFocus: true }); + + // then + function onFocus() { + try { + const node = findRenderedDOMElementWithClass(renderedTree, 'context-menu'); + + expect(node).to.exist; + expect(node.style.top).to.not.equal(''); + expect(node.style.left).to.not.equal(''); + + done(); + } catch (error) { + done(error); + } + } + } + ); + + test(); + }); + + + + it('should ignore contextMenu.open if no components', inject( function(contextMenu, eventBus, injector) { From 6e21af208ce78365ea306daea0ac609d2a7e7346 Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Wed, 23 Oct 2019 11:31:24 +0200 Subject: [PATCH 2/2] test(context-menu): skip focus test on Firefox Firefox inconsistently fires focus events which causes the tests to fail but in real usage autofocus works correctly. Cf. https://github.com/bpmn-io/table-js/pull/25 and linked sources --- .../components/ContextMenuComponentSpec.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/spec/features/context-menu/components/ContextMenuComponentSpec.js b/test/spec/features/context-menu/components/ContextMenuComponentSpec.js index d9eaaaf..478bdbe 100644 --- a/test/spec/features/context-menu/components/ContextMenuComponentSpec.js +++ b/test/spec/features/context-menu/components/ContextMenuComponentSpec.js @@ -234,7 +234,9 @@ describe('features/context-menu - ContextMenuComponent', function() { })); - it('on open', inject(function(contextMenu) { + // skip on Firefox due to inconsistent focus handling, + // cf. https://github.com/bpmn-io/table-js/pull/25 and linked sources + (isFirefox() ? it.skip : it)('on open', inject(function(contextMenu) { // when contextMenu.open(); @@ -340,8 +342,11 @@ describe('features/context-menu - ContextMenuComponent', function() { } )); - - it('should set position before the context menu is focused', function(done) { + // skip on Firefox due to inconsistent focus handling, + // cf. https://github.com/bpmn-io/table-js/pull/25 and linked sources + ( + isFirefox() ? it.skip : it + )('should set position before the context menu is focused', function(done) { const test = inject( function(components, contextMenu, eventBus, injector) { @@ -468,4 +473,10 @@ function triggerEvent(element, name, eventType, bubbles=false) { function triggerFocusIn(element) { return triggerEvent(element, 'focusin', 'UIEvents', true); +} + +function isFirefox() { + return window.navigator && + window.navigator.userAgent && + /Firefox/.test(window.navigator.userAgent); } \ No newline at end of file