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

GUI: Minor fixes (scrollbars, tooltips, tabs, EditText) #1785

Merged
merged 4 commits into from Aug 3, 2019

Conversation

@Tkachov
Copy link
Contributor

commented Aug 2, 2019

I've been making a custom theme based on Modern Remastered these couple of days (I call it Midnight theme). While I was at it, I locally fixed some things in GUI. Well, some changes are not really fixes, but edits that I thought look better.

These are not changes for my theme only, but sometimes they are better illustrated with my theme (scrollbars, for example). Below are pics comparing what was before the fixes and after these in both my theme and Modern Remastered (click to view in full size).

15ee60f

  • list widget scrollbar is not cropped on the right now:

01_cropped_scrollbar_in_list_widget fw

.

  • scrollbar buttons now take as much space as they should, not overlapping with scrollbar handle:

02_scrollbar_buttons_size fw

(illustrated in Midnight theme only, because Modern Remastered does not have any background or borders in scrollbar buttons, so you cannot see the overlapping)

95b8c99

  • scrollbar repositioned in ScrollContainerWidget, and widget itself is now drawn so its bottom border goes lower than clipped contents:

03_scroll_container fw

(unfortunately, I couldn't resize scrollbar to have the same height as container for my Midnight theme, because then in Modern Remastered it overlap tabs not 1px, but 2px. As a result, we can see these "double borders" in Midnight)

(we can see that "tab" drawstep behaves strangely, and draws 2 more pixels where ScrollContainerWidget's area already begins; using "square" drawstep in Midnight theme solves that and looks more tidy)

be1e8ae

  • added 2px padding in tooltip:

04_tooltip fw

.

  • shifted text in EditTextWidget 1px up, so it's more vertical aligned:

05_edittext fw

7dda053

  • fixed TabWidget's < and > buttons to use values from the theme:

06_tab_buttons fw

.

And a long pic for lowres:

07_lowres fw

.

So, 15ee60f and 7dda053 are actual fixes (cropped scrollbar, TabWidget typos) and 95b8c99 with be1e8ae are just my edits that look prettier (vertical aligned text in EditText, 2px margin in tooltip, repositioned scrollbar).

P.S. Midnight theme is available on my site

Tkachov added some commits Jul 30, 2019

GUI: Fix scrollbars
- removed +1px in ListWidget, added in lordhoto's 2007 commit 68eb28a
(aka r29971 in svn) `Fix for bug #1670082 "GUI: Modern theme gfx glitch
in launcher".`, because it made clip this last line of scrollbar in all
themes, which doesn't look good. In 2007 theme was written in .ini,
which is not the case now. I don't see any glitches after removing this
"fix";

- fixed how scrollbar top and bottom scroll buttons are drawn in
ThemeEngine::drawScrollbar: there were these weird magic numbers, but in
reality extra space that buttons should occupy is hardcoded in
scrollbar.cpp (ScrollBarWidget) and is just +1px.
GUI: Tune ScrollContainerWidget offsets
Well, it ain't a fix, because it's not exactly correct for any of the
themes. Yet it's the best for all of them. If I put what seems to be
correct, "modern" theme gets ruined, because it has this mystical 2px
offset in tabs/scrollcontainers.
GUI: Fix TabWidget's < and > buttons
These were incorrectly positioned (typos in code, missing value in one
expression).
@Mataniko

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

Looks great, thank you for doing this.

I really like the scrollbar in the midnight theme, maybe we should bring that over to the remastered theme....

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

I really like the changes! Subtle, but quite welcome! The buttons after the tabs are much much nicer now

@criezy

criezy approved these changes Aug 3, 2019

Copy link
Member

left a comment

Thank you!
Those indeed look better with your changes. The shift in the EditTextWidget also improve the issue that the bottom of some letters (such as p or q) is cut and no longer appears to be cut after your change.
Before:
image
After:
image

For reference, since I saw this while testing, there is an issue with the content of the ScrollWidgetContainer overlapping the scrollbar, at least in the classic theme, when the focus changes. This is however already the case in current master and is not something introduced with this pull request.
image

@bluegr

bluegr approved these changes Aug 3, 2019

@Tkachov

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2019

@criezy , about that content overlapping: I guess this could/should be fixed by adding more right padding in the layout. That's a workaround, but if this glitch happens because of overlapping, I think the easiest way to fix it is just make things not overlap.

@Mataniko

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

@Tkachov before merging, can you run scummvmtheme.py?

@Tkachov

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2019

@Mataniko I sure can, but why is that needed? These changes are in code, not in themes layout.

@Mataniko

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

My bad, I thought there was an XML change too. Merging.

@Mataniko Mataniko merged commit 0a3d6d8 into scummvm:master Aug 3, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.