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

Improve the height of plugins in the right split #194

Merged
merged 3 commits into from
Mar 18, 2021

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

Summary

The height of plugins stacked on the right split doesn't always update in a reasonable way. This pull request improves the situation. The following situations use examples/config/layout.config.

  • If the window is not large enough to fit all plugins, squish them all to their minimum height and make the split scrollable.
  • If the window is large enough to fit all plugins, they are stretched to fill the entire height, and their proportion can be changed dragging the separator.
  • Collapsed plugins behave consistently with the above
  • Adding / removing plugins also behaves as expected

split1

split2

split3

Checklist

  • Signed all commits for DCO
  • Added tests: No, because we don't have a good way of testing the view right now 😕
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #194 (b1daa0b) into ign-gui3 (071a8e4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           ign-gui3     #194   +/-   ##
=========================================
  Coverage     59.03%   59.03%           
=========================================
  Files            23       23           
  Lines          2739     2739           
=========================================
  Hits           1617     1617           
  Misses         1122     1122           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 071a8e4...b1daa0b. Read the comment docs.

@chapulina chapulina added the bug Something isn't working label Mar 16, 2021
@scpeters scpeters mentioned this pull request Mar 18, 2021
7 tasks
@scpeters
Copy link
Member

I don't really get what's supposed to happen, and I am having trouble reproducing the behavior in your animations

@scpeters
Copy link
Member

I don't really get what's supposed to happen, and I am having trouble reproducing the behavior in your animations

I see that the separator is not movable with the current version, but it is movable with this branch

@scpeters
Copy link
Member

I don't really get what's supposed to happen, and I am having trouble reproducing the behavior in your animations

I see that the separator is not movable with the current version, but it is movable with this branch

but I don't consistently see the auto-adjusting of size that you mention in the description

@chapulina
Copy link
Contributor Author

I should have mentioned the issue that this is trying to solve. Without this PR, the plugins are always squished to their minimum heights and don't expand to take up the free space. For example, see how the entity tree is small even though there's a lot of empty space under it:

image

This also means that the separators are never draggable without this PR, because the widgets are fixed to their minimum heights.

@chapulina
Copy link
Contributor Author

This issue was brought up here, see how the widget is squished there: gazebosim/gz-sim#534 (review)

@scpeters
Copy link
Member

  • If the window is large enough to fit all plugins, they are stretched to fill the entire height, and their proportion can be changed dragging the separator.

I think I was misinterpreting what you meant by this. I see the same behavior as in your animations now, so it looks fine enough to me, even if I'm not sure what "stretched to fill the entire height" means

@chapulina
Copy link
Contributor Author

I'm not sure what "stretched to fill the entire height" means

You can see in my gifs that unless the plugins are collapsed, there's never empty space below them. They are "stretched" to fill the entire column.

Thanks for the quick review!

@chapulina chapulina merged commit 3794b8a into ign-gui3 Mar 18, 2021
@chapulina chapulina deleted the chapulina/3/split_height branch March 18, 2021 06:15
scpeters added a commit that referenced this pull request Mar 18, 2021
Signed-off-by: Steve Peters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants