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

Layout tweaks #1317

Merged
merged 9 commits into from
Apr 13, 2020
Merged

Layout tweaks #1317

merged 9 commits into from
Apr 13, 2020

Conversation

scribblemaniac
Copy link
Member

This PR tweaks a few parts of the display and tools dock widgets' appearance. These include:

  • Updating the minimum height of the widgets which use FlowLayout on resize so that they cannot be resized in such a way that hides tools or other items.
  • Use FlowLayout instead of GridLayout for the display options widget
  • Add support for horizontal centering to FlowLayout. I currently only have this enabled for the display widget as it looks a little weird to me when the tools are not left-aligned like in all other programs.
  • Added a spacer to the bottom of the onion skins dock widget just so it behaves like the tool options widget

And here are some visuals

FlowLayout Resizing:
Toolbox Resizing 1
Toolbox Resizing 2

Horizontal centering:
Display Centering

Note that if you move a widget with FlowLayout from floating to a dock with a smaller width, it may overflow into the widget below. I have not found a fix for this, but I don't think it is very high priority since interacting with the widget in any way will trigger some update which fixes the issue.

Also note that this has only been tested on Linux. Given the slightly hackish nature of this solution, it is possible that it will not work on some other operating system. We should test Mac and Linux before applying these changes.

@scribblemaniac scribblemaniac added UI Related to the visual appearance of the program 🔹 Minor PR (only one reviewer required) labels Mar 29, 2020
@scribblemaniac scribblemaniac added this to the 0.6.5 milestone Mar 29, 2020
@MrStevns MrStevns self-assigned this Mar 29, 2020
Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM and works fine on mac, the only 'issue' I saw is that when the toolbox widget is floating and I stretch it to fill one row, there's almost spacing enough for two rows below it.

image

It won't get smaller than this unless I dock it.

edit: ah nvm... this is not new, it's an old thing.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM, tested after latest changes and looks 'correct' on mac now, well done 👍

Would be nice to move that resize chunk that's repeated inside BaseDockWidget but I can see we use a non generic ui element each time... couldn't we pass the height in as a param to the BaseDockWidget? that's a bit better than having this repeated per time we need to use the flowLayout.

@scribblemaniac
Copy link
Member Author

Would be nice to move that resize chunk that's repeated inside BaseDockWidget but I can see we use a non generic ui element each time... couldn't we pass the height in as a param to the BaseDockWidget? that's a bit better than having this repeated per time we need to use the flowLayout.

Yeah it would be a good idea to make a helper function for that in BaseDockWidget. I'll try that soon.

@scribblemaniac
Copy link
Member Author

Alright, this is refactored and ready to go now hopefully.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM, merging 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔹 Minor PR (only one reviewer required) UI Related to the visual appearance of the program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants