这是indexloc提供的服务,不要输入任何密码
Skip to content

Make OnionSkins UI more compact #1824

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

Closed
wants to merge 1 commit into from

Conversation

chchwy
Copy link
Member

@chchwy chchwy commented Apr 19, 2024

Improve the OnionSkins UI by reducing the spacing between UI components.

Currently, on my HiDPI laptop displays (Win11 with 150% scaling), the OnionSkins options are barely visible. We can improve it by minimizing the spacing between UI elements, so more options can be displayed within the limited available space.

Please see the before/after screenshots as follows:

Before

onionskins-old

After

onionskins-new

Copy link
Member

@scribblemaniac scribblemaniac left a comment

Choose a reason for hiding this comment

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

While I agree this change is an improvement on Windows, it doesn't look as good on Linux/GNOME. On the left is your version, the right is the old version (the font is slightly different in the AppImage apparently):
Screenshot from 2024-04-21 11-52-59
Because the border is drawn underneath the group box text instead of through it, it looks to me like there isn't enough padding on the top. I think we should add 3px to the LayoutTopMargin of nextFramesGroup and prevFramesGroup so that it's evenly spaced on Linux. While OS specific code could be added, I don't think it's worth it for 6px.

@scribblemaniac scribblemaniac added Enhancement UI Related to the visual appearance of the program 🔹 Minor PR (only one reviewer required) labels Apr 21, 2024
@chchwy
Copy link
Member Author

chchwy commented Apr 22, 2024

While I agree this change is an improvement on Windows, it doesn't look as good on Linux/GNOME. On the left is your version, the right is the old version (the font is slightly different in the AppImage apparently) Because the border is drawn underneath the group box text instead of through it, it looks to me like there isn't enough padding on the top. I think we should add 3px to the LayoutTopMargin of nextFramesGroup and prevFramesGroup so that it's evenly spaced on Linux. While OS specific code could be added, I don't think it's worth it for 6px.

thanks for testing. I will tweak the spacing to see if I can make it look better.

@chchwy
Copy link
Member Author

chchwy commented Apr 22, 2024

BTW, this not only improves the Windows app but also enhances the Mac app, which by default uses a HiDPI display

Original
old

Compact
compact

@J5lx
Copy link
Member

J5lx commented Apr 22, 2024

Adding to what scribble mentioned, this also doesn’t look good when the border is drawn above the group box text, such as with KDE’s default theme Breeze. Before:

image

After:

image

@scribblemaniac
Copy link
Member

Maybe we shouldn't use QGroupBox here if the display is so inconsistent and keeping the layout compact is a priority?

@J5lx
Copy link
Member

J5lx commented Apr 25, 2024

Maybe we shouldn't use QGroupBox here if the display is so inconsistent and keeping the layout compact is a priority?

I don’t think this is something we should even have to worry about. To be completely honest, I’m not entirely convinced of this micromanagement of spacing and margin values in the first place. This will shave off a few pixels now, but the moment we make more meaningful changes (like the addition of an onion skin global toggle), we’ll be back to square one. If we’re really that concerned about keeping all the stuff on screen, we should just arrange (some of) the tool windows horizontally next to each other by default, rather than stacking them all vertically, or reduce the overall UI scale, or lock the tool windows by default in order to conserve the space consumed by their titlebars. I worry that if we instead choose to micromanage our pixels like this we’ll just end up dealing with more edge-cases like these groupboxes in the future.

@chchwy chchwy force-pushed the feat/compact-onionskins branch from 13e652b to 5452aaa Compare April 26, 2024 09:14
@chchwy
Copy link
Member Author

chchwy commented Apr 26, 2024

@J5lx @scribblemaniac I've adjusted the margins, could you please test it again?

@J5lx
Copy link
Member

J5lx commented Apr 26, 2024

Looks better now

image

@MrStevns
Copy link
Member

MrStevns commented Apr 26, 2024

It's not quite there imo, the padding is off on macOS.
image

I agree with Jacob that trying to sub-optimize this to shave off a few pixels is a bit silly, when we should instead consider either rethinking it or creating a custom UI that can be made smaller.

Spacing should be consistent between our widgets, anything else is a UI blunder if you ask me.
image

This, as it was, is better because it's aligned and there's space. Padding is important to make UI not feel cramped.
image

@chchwy chchwy closed this Apr 27, 2024
@chchwy chchwy reopened this Apr 27, 2024
@chchwy
Copy link
Member Author

chchwy commented Apr 27, 2024

It's not quite there imo, the padding is off on macOS. image

I don't see obvious problems in this screenshot. can you elaborate more?

I agree with Jacob that trying to sub-optimize this to shave off a few pixels is a bit silly, when we should instead consider either rethinking it or creating a custom UI that can be made smaller.

Spacing should be consistent between our widgets, anything else is a UI blunder if you ask me. image

This, as it was, is better because it's aligned and there's space. Padding is important to make UI not feel cramped. image

May I ask what your screen size is and the resolutions? The discussion will go in the wrong direction if we only talk about aesthetic. The change here want to strike a balance between the aesthetic and usability with HiDPI display.

@chchwy
Copy link
Member Author

chchwy commented Apr 27, 2024

Maybe we shouldn't use QGroupBox here if the display is so inconsistent and keeping the layout compact is a priority?

I don’t think this is something we should even have to worry about. To be completely honest, I’m not entirely convinced of this micromanagement of spacing and margin values in the first place. This will shave off a few pixels now, but the moment we make more meaningful changes (like the addition of an onion skin global toggle), we’ll be back to square one. If we’re really that concerned about keeping all the stuff on screen, we should just arrange (some of) the tool windows horizontally next to each other by default, rather than stacking them all vertically, or reduce the overall UI scale, or lock the tool windows by default in order to conserve the space consumed by their titlebars. I worry that if we instead choose to micromanage our pixels like this we’ll just end up dealing with more edge-cases like these groupboxes in the future.

I don't really agree that space reduction is micro-management.

The things you mentioned above, the global onion toggle and horizontal tools options, they are all valuable and should be considered seriously. However, these things do not conflict with the space reduction.

If you look at other tools such as Photoshop, Paint Tool SAI or Procreate, you may notice that they are extremely reluctant to put spaces between UI components in order to make the most use of limited screen spaces.

I want to emphasize again that I want to improve the usability of HiDPI displays as they are becoming more and more common and our current UI usability with HiDPI displays is quite poor.

@MrStevns
Copy link
Member

MrStevns commented Apr 27, 2024

May I ask what your screen size is and the resolutions? The discussion will go in the wrong direction if we only talk about aesthetic. The change here want to strike a balance between the aesthetic and usability with HiDPI display.

My resolution is technically (2560 × 1600) retina but I have scaled UI, so it appears to be 1440x900. The screen is 13 inches.

I understand that you want to strike a balance between aesthetic and usability, which is fine, as long as it's consistent across our widgets. For example if we should have 2 or 3 px instead of 6 px padding everywhere, then that's fine too, as long as it's consistent.

I don't see obvious problems in this screenshot. can you elaborate more?

You can see the padding difference here:
image

Looking at the code, the padding is 3 px around the overall group, yet there appears to be more spacing on the right side.
From a bit of digging, it seems to come from the checkboxes not being placed in a layout.

The interesting thing here is that if I tweak the spacing, the font size and try to squish the layout as much as possible, the layout still ends up being wider than your the changes you made, which just shows that tweaking this has all kinds of weird outcomes.

image

Top: My tweaked layout (wider)
Behind: Your tweaked layout (narrower)

When I compare our layout with my CSP layout, ours is 25-30 px wider, but you can also see that they do all kinds of trickery to get it smaller, like squishing the font, potentially hiding it partially behind UI, having generally a more compact layout by more or less embedding the description into the slider component.

CSP Pencil2D (chchwy) Pencil2D (MrStevns)
image image image
165 px 188 px 192 px

Aha...! the layout designer sets a minimum width on the outer layout if none is given, which apparently is 192 px in my case. Changing this manually, we could probably go down to 140 pixels and it would look alright, that's of course with the tweaked font size too. Would that be sufficient?

11 point font size 13 point font size
image image
142 px 162 px

OBS: If we do agree to changing the font size, then it should be done throughout our dock widgets 😄

After going through all widgets, we could end up with something like this:

image Front: my tweaked layout:
  • Most dock widgets has a min width of 140 px or less
  • Font size is now 11 point instead of 13.
  • Padding has been adjusted to be 3 px across all the dock widgets.
    Back: your layout

Overall outcome:
image

Do you want me to make a PR with this?

I've made these changes available on: https://github.com/MrStevns/pencil/tree/compact-dock-layout

@chchwy
Copy link
Member Author

chchwy commented Apr 27, 2024

Thanks a lot for the efforts. I was thinking of starting small from a single widget and then step by step making the changes to other widgets. But you have done them all! That's fantastic. I will definitely check your branch out when I get time.

@chchwy
Copy link
Member Author

chchwy commented Apr 27, 2024

BTW, I also thinking of replacing the title bar of dock widgets. The default title bars eat a lot of spaces.

@MrStevns
Copy link
Member

MrStevns commented Apr 27, 2024

I agree, changing the titlebar widgets or getting rid of them entirely in docked mode is also something I've experimented with during my early timeline mockups. Integrating the close and popup button directly into the given widget would make the UI a lot more compact vertically, on mac os at least.

this is a mockup from 2020, so the style does not fit anymore but the concept itself could probably still be applied.
image

Although remember that you can technically already do that, by locking the UI.
image

That's a separate topic though :)

@chchwy
Copy link
Member Author

chchwy commented Apr 29, 2024

Do you want me to make a PR with this?
I've made these changes available on: https://github.com/MrStevns/pencil/tree/compact-dock-layout

Yes, please. Your branch is overall better than mine. I am going to close this PR.

@chchwy chchwy closed this Apr 29, 2024
@chchwy chchwy deleted the feat/compact-onionskins branch June 4, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement 🔹 Minor PR (only one reviewer required) UI Related to the visual appearance of the program
Projects
Status: Discarded
Development

Successfully merging this pull request may close these issues.

4 participants