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.

Sign up for GitHub

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

Jump to bottom

Minimize GUI Plugins #84

Merged
merged 9 commits into from
Aug 1, 2020
Merged

Minimize GUI Plugins #84

merged 9 commits into from
Aug 1, 2020

Conversation

Sarath18
Copy link
Contributor

@Sarath18 Sarath18 commented Jun 19, 2020 •
edited
Loading

Signed-off-by: Sarathkrishnan Ramesh [email protected]

Reference to issue #74

@Sarath18
Copy link
Contributor Author

The PR is currently in draft state. I am working on minimizing the gui plugins when in the docked state.

@chapulina chapulina added the 📜 blueprint Ignition Blueprint label Jun 23, 2020
@Sarath18
Copy link
Contributor Author

Added two new states d_collapsed (docked collapsed) state and f_collapsed (floating collapsed) state and transition between these and the existing states. These states are working on config load as well.

min_plugin

@Sarath18
Minor updates
6597173 
* Handle case when user manually minimizes plugin using resize

* Replace nested if with switch case for better readability

Signed-off-by: Sarathkrishnan Ramesh <[email protected]>
@Sarath18 Sarath18 marked this pull request as ready for review July 13, 2020 15:29
@Sarath18 Sarath18 requested a review from chapulina as a code owner July 13, 2020 15:29
@chapulina chapulina linked an issue Jul 14, 2020 that may be closed by this pull request
Minimize plugins #74
Closed
chapulina
chapulina reviewed Jul 20, 2020
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Nice!

I noticed that the last docked plugin can't be minimized. Did you notice that? It looks like the internal state is collapsed, even though the icon doesn't change, because the height isn't changing.

For completeness, we should make the button configurable from the settings dialog like the other buttons:

https:/ignitionrobotics/ign-gui/blob/c5380098624b48e422cf82c16a474af9afccad59/include/ignition/gui/qml/IgnCardSettings.qml#L44-L64

It would also be nice to add a collapsed example to examples/config/plugin_params.config.

include/ignition/gui/qml/IgnCard.qml Outdated Show resolved Hide resolved
include/ignition/gui/qml/IgnSplit.qml Outdated
@@ -328,6 +328,8 @@ SplitView {
{
Layout.minimumWidth = child.Layout.minimumWidth;
}
// Set child height to minimu height
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Set child height to minimu height
// Set child height to minimum height

include/ignition/gui/qml/IgnCard.qml Outdated Show resolved Hide resolved
@Sarath18
Minor updates
f264ca1 
* Rename minimize to collapse

* Rename states

* Add switch in card settings to enable/disable collapse button

* Add a collapsed plugin to plugin_params.config

Signed-off-by: Sarathkrishnan Ramesh <[email protected]>
@Sarath18
Copy link
Contributor Author

I noticed that the last docked plugin can't be minimized. Did you notice that? It looks like the internal state is collapsed, even though the icon doesn't change, because the height isn't changing.

Yes, you are right. I looked at it again and found that this case only happens when the sum of minimum heights of the plugins loaded (in split) is less than the height of the app window. If we have many expanded plugins before the last collapsed plugins, the expand and collapse work just fine. But the weird part is if you close the left split and try to expand/close the last (or any) plugins, it works as intended. Here is a gif describing the above-mentioned details.

min_bug

I am not sure why is this happening, can you help me find the issue?

@Sarath18
Fix last plugin collapse in docked state
95670a7 
Signed-off-by: Sarathkrishnan Ramesh <[email protected]>
@Sarath18 Sarath18 requested a review from chapulina July 27, 2020 13:16
@chapulina
Copy link
Contributor

can you help me find the issue?

Sure, I'll take a look this week 😉

@Sarath18
Copy link
Contributor Author

Sure, I'll take a look this week 😉

Thank you for the help, but I believe I fixed it in the last commit 95670a7. Can you please review it?

chapulina
chapulina reviewed Jul 31, 2020
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I like how you handled the height of the last docked plugin, great job 👍

I just have one last question about undocking while collapsed.

include/ignition/gui/qml/IgnCard.qml Outdated Show resolved Hide resolved
@Sarath18 Sarath18 requested a review from chapulina July 31, 2020 16:18
chapulina
chapulina approved these changes Aug 1, 2020
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Works great! I just pushed a little fix that I noticed on 3483451.

Will merge when CI is back.

@chapulina chapulina merged commit b2bb0e4 into gazebosim:ign-gui2 Aug 1, 2020
@Sarath18 Sarath18 mentioned this pull request Aug 1, 2020
Merged
@Sarath18 Sarath18 deleted the feature_minimize_plugins branch August 5, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

Assignees
No one assigned
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Minimize plugins
2 participants
@Sarath18 @chapulina