Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize fix for swallowed hover events #503

Merged
merged 1 commit into from
Dec 1, 2020
Merged

Generalize fix for swallowed hover events #503

merged 1 commit into from
Dec 1, 2020

Conversation

azeghers
Copy link

@azeghers azeghers commented Nov 29, 2020

Replaces the old drag hover fix with one that works in a general fashion only on element.* level.

Related to bpmn-io/bpmn-js#1366

Acceptance Criteria

  • Corresponds to the concept
  • Corresponds to the design

Definition of Done

@azeghers azeghers requested review from a team, pinussilvestrus and barmac and removed request for a team November 29, 2020 14:36
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Nov 29, 2020
@azeghers azeghers force-pushed the hover-fix branch 3 times, most recently from 29c58d4 to eee7816 Compare November 29, 2020 14:43
lib/features/hover-fix/index.js Outdated Show resolved Hide resolved
test/spec/features/hover-fix/HoverFixSpec.js Outdated Show resolved Hide resolved
@barmac
Copy link
Member

barmac commented Nov 30, 2020

Closes bpmn-io/bpmn-js#1366

When we create a solution for an upstream issue, the fix does not solve the issue yet. It will be the update of the dependency which solves the problem and the fix can only be Related to bpmn-io/bpmn-js#1366. For example, have a look at camunda/camunda-modeler#1997

@barmac
Copy link
Member

barmac commented Nov 30, 2020

I've tested this on my machine and it works correctly when the new module is included in bpmn-js.

@azeghers
Copy link
Author

I have made the HoverFixModule a dependency of the DraggingModule, so that anything that imports it initializes the hover fix in the process. This should implicitly enable it everywhere, so long as the dragging module is used.

@azeghers
Copy link
Author

Additionally, the tests should now also ensure that the element-level hover fix doesn't need the dragging module bootstrapped to work, and that the dragging-level hover fix doesn't need the hover fix module bootstrapped to work.

Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing and we got it, I believe!

test/spec/features/create/CreateSpec.js Outdated Show resolved Hide resolved
Replaces the old drag hover fix (which fixed kindof the same thing)
with one that works in a general fashion only on element.* level.

Related to bpmn-io/bpmn-js#1366
@nikku nikku changed the base branch from master to develop December 1, 2020 13:52
@nikku nikku merged commit eacd65c into develop Dec 1, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Dec 1, 2020
@fake-join fake-join bot deleted the hover-fix branch December 1, 2020 13:53
@nikku
Copy link
Member

nikku commented Dec 1, 2020

🆒

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants