-
Notifications
You must be signed in to change notification settings - Fork 94
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
gcylc: dot view - maintain user selection during update #2221
gcylc: dot view - maintain user selection during update #2221
Conversation
15c8c6e
to
bcecd46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question.
Tested as works as intended, as long as the selected task is still around.
lib/cylc/gui/updater_dot.py
Outdated
Warning: This method has not been tested with multiple selection. | ||
|
||
""" | ||
def should_select_row(model, path, iter_, selection): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why this function/method needs to be embedded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic doesn't really make sense within DotUpdater so I encapsulated it (pep227). I could convert it to a lambda if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The method, however, does refer to self
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this is resolved happy for this to be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewrmshin Code now moved into a static method.
(sorry - didn't notice I was not assigned as a reviewer 😬) |
Doubly approved already, so I think we can remove @arjclark as reviewer. |
Closes #1707
Doesn't go as far as Ben's incremental build suggestion (#1707 (comment)).