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

Prevent mutation menu closing when clicking on another task or cycle #927

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Feb 15, 2022

These changes close #916

demo6

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (e2e).
  • Appropriate change log entry included in Prepare release: 1.0.0 #935
  • No documentation update required.

@MetRonnie MetRonnie added the bug Something isn't working label Feb 15, 2022
@MetRonnie MetRonnie added this to the 1.0.1 milestone Feb 15, 2022
@MetRonnie MetRonnie self-assigned this Feb 15, 2022
@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #927 (73958fb) into master (af4ae02) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #927      +/-   ##
==========================================
+ Coverage   90.30%   90.57%   +0.27%     
==========================================
  Files          92       92              
  Lines        2000     2005       +5     
  Branches      151      152       +1     
==========================================
+ Hits         1806     1816      +10     
+ Misses        168      163       -5     
  Partials       26       26              
Flag Coverage Δ
e2e 76.90% <100.00%> (+0.45%) ⬆️
unittests 78.25% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/components/cylc/cylcObject/Menu.vue 77.77% <100.00%> (+1.91%) ⬆️
src/components/cylc/cylcObject/plugin.js 100.00% <100.00%> (ø)
src/utils/aotf.js 95.93% <0.00%> (+2.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af4ae02...73958fb. Read the comment docs.

el.removeEventListener('show-mutations-menu', listener)
el.removeEventListener('click', listener)
Copy link
Member Author

@MetRonnie MetRonnie Feb 16, 2022

Choose a reason for hiding this comment

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

I think this was a mistake, the first arg should be the event that you're removing the listener for

Copy link
Contributor

@AaronDCole AaronDCole left a comment

Choose a reason for hiding this comment

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

Seems to work as expected!

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Looks sensible, and works for me too.

I'll leave it for @wxtim to merge (sounds like he wants to look at the code more closely) [Oops wrong PR for that statement!]

@hjoliver hjoliver merged commit 8dd7e52 into cylc:master Feb 16, 2022
@MetRonnie MetRonnie deleted the mutation-menu branch February 17, 2022 10:29
@oliver-sanders oliver-sanders modified the milestones: 1.1.0, 1.0.0 Feb 17, 2022
@MetRonnie MetRonnie mentioned this pull request Feb 17, 2022
2 tasks
@MetRonnie MetRonnie mentioned this pull request May 2, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking on another cycle or task icon causes mutation menu to show and dismiss instantly
4 participants