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

Fix: Maximize Volume Pane on Start Navigation #844

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Hafsa-shoaib989
Copy link
Contributor

This PR fixes issue #835 where the volume pane was not being maximized upon starting the navigation.

Changes Made:
Added logic to send a maximize_volume_pane message when navigation starts in default_viewers,py file.
Modified the OnStartNavigationButton function in task_navigator.py file.

Testing:
Tested and verified that the volume pane now automatically maximizes when navigation is initiated.

Hi @henrikkauppi @vhosouza @tfmoraes please review this new Pull request.

@henrikkauppi
Copy link
Collaborator

Clicking the start navigation button maximizes the volume viewer but I found a bug with this implementation.

Problems:

  • if the volume viewer is already maximized when clicking the start navigation button, then it is not possible to manually restore the volume viewer window to its original smaller size by clicking the restore icon at the top right corner of the window. For example, starting navigation, stopping it, and starting navigation again reproduces this bug. Another way to trigger it is to manually maximize the window and then click start navigation.

Also, please use pre-commit in the future. Instructions on setup and use can be found here.

@henrikkauppi
Copy link
Collaborator

There's still a bug where if you start navigation while the volume window is maximized, it is not possible to minimize the volume window no matter what you do after that.

@rmatsuda
Copy link
Collaborator

Hi guys,
Thanks for the PR @Hafsa-shoaib989!
Luka´s PR (#827) is performing the maximize after the target mode is set.
I recommend to wait until this PR is accepted, and then check the need of improvements.

@henrikkauppi
Copy link
Collaborator

Hi guys, Thanks for the PR @Hafsa-shoaib989! Luka´s PR (#827) is performing the maximize after the target mode is set. I recommend to wait until this PR is accepted, and then check the need of improvements.

I agree, let's wait until Luka's PR is merged.

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.

3 participants