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

gcylc: dot view - maintain user selection during update #2221

Conversation

oliver-sanders
Copy link
Member

Closes #1707

Doesn't go as far as Ben's incremental build suggestion (#1707 (comment)).

@oliver-sanders oliver-sanders added this to the next release milestone Mar 28, 2017
@oliver-sanders oliver-sanders force-pushed the 1707.dot-view-redraw-reseting-selection branch from 15c8c6e to bcecd46 Compare March 28, 2017 11:56
Copy link
Contributor

@matthewrmshin matthewrmshin left a 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.

Warning: This method has not been tested with multiple selection.

"""
def should_select_row(model, path, iter_, selection):
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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.)

Copy link
Contributor

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.

Copy link
Member Author

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.

@hjoliver
Copy link
Member

hjoliver commented Mar 28, 2017

(sorry - didn't notice I was not assigned as a reviewer 😬)

@matthewrmshin
Copy link
Contributor

Doubly approved already, so I think we can remove @arjclark as reviewer.

@matthewrmshin matthewrmshin removed the request for review from arjclark March 30, 2017 10:26
@matthewrmshin matthewrmshin merged commit acebd2f into cylc:master Mar 30, 2017
@oliver-sanders oliver-sanders deleted the 1707.dot-view-redraw-reseting-selection branch December 14, 2017 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants