Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
active_op_id needs to be shared #2326
active_op_id needs to be shared #2326
Changes from 1 commit
85b40e0
1ff7106
f963a1a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
In which situation should the topview's active_op_id be something different than the mainwindow.mscolab.active_op_id? I.e. why does the topview need a copy of this attribute, instead of always using the one from the mainwindow?
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.
topview won't need it. The current implementation of the multiple_flighpath_dockingwidget has no access to the mainwindow, e.g.
https://github.com/Open-MSS/MSS/blob/develop/mslib/msui/multiple_flightpath_dockwidget.py#L507
There were more vars introduced which are already known in the mainwindow https://github.com/Open-MSS/MSS/blob/develop/mslib/msui/topview.py#L204
Currently I do prefer first a kind of logic test coverage before we refactor the inner. It is somehow improving the Proof-Of-Concept before we can do the cleanup.
One of the bigger refactoring steps is to remove all the dups and focus on a better understandable API.
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.
Couldn't the topview just hold a reference to the mainwindow then? Holding another reference to all of the mainwindow's signals is also something that feels wrong to me, and could be eliminated if the mainwindow was just accessible through the topview.
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.
I think that would be a better solution and has to be done in a refactoring. @Jatin2020-24 did you have a reason not to do it that way?
This PR is related to fix a bug in the order for opening the dockingwidget when a mscolab operation was selected. We need a few more tests to figure that any combination for the docking widget works.
I guess there is at least one more bug which needs to be test covered and fixed.
When this part is refactored we also have to refactor tests.