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: Embiggen the Options dialogs #3976

Merged
merged 1 commit into from Jun 11, 2022
Merged

GUI: Embiggen the Options dialogs #3976

merged 1 commit into from Jun 11, 2022

Conversation

SupSuper
Copy link
Contributor

@SupSuper SupSuper commented Jun 7, 2022

ScummVM has grown a lot of options over the years. Maybe the dialog should grow as well?

Before:
image

After:
image

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Jun 7, 2022

Very nice. I think this makes way more sense to have the modal covering up the buttons - they are not usable anyways and just waste space.

+1.

@bluegr
Copy link
Member

@bluegr bluegr commented Jun 7, 2022

Very nice! The end result is much much better indeed!
+1 from me too

bluegr
bluegr approved these changes Jun 7, 2022
@lotharsm
Copy link
Member

@lotharsm lotharsm commented Jun 7, 2022

I wonder if the THEMERC version needs to be bumped here as well? Or is a version bump only necessary if new widgets/elements are introduced?

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Jun 8, 2022

Considering all the "thumbs up" for this PR - what do you think about backporting this change to branch-2-6? It really looks like a nice QoL improvement with no risk of unwanted side effects.

@SupSuper
Copy link
Contributor Author

@SupSuper SupSuper commented Jun 8, 2022

I wonder if the THEMERC version needs to be bumped here as well? Or is a version bump only necessary if new widgets/elements are introduced?

The latter, this is just a cosmetic change so it won't break theme compatibility.

Considering all the "thumbs up" for this PR - what do you think about backporting this change to branch-2-6? It really looks like a nice QoL improvement with no risk of unwanted side effects.

I guess @sev- should be the final judge of that. :)
Shouldn't break anything but I'll wanna double-check if all the dialogs match first.

@bluegr
Copy link
Member

@bluegr bluegr commented Jun 11, 2022

Nice work! I've tested this and it looks great.
Let's merge it in master so that we can test it a bit more, and if there are no regressions, it should be good to go into 2.6.0 as well.

@bluegr bluegr merged commit 7acc1cd into scummvm:master Jun 11, 2022
8 checks passed
@sev-
Copy link
Member

@sev- sev- commented Jun 11, 2022

I am not a fan of this change as it breaks the design concept we use.

@sev-
Copy link
Member

@sev- sev- commented Jun 11, 2022

Also, how does it look in fullscreen?

@sev-
Copy link
Member

@sev- sev- commented Jun 11, 2022

I am resisting to call it "nice"

Screenshot 2022-06-11 at 19 36 35

@sev-
Copy link
Member

@sev- sev- commented Jun 11, 2022

The setting is fine for the demonstrated 640xY resolution, but at some point, it should be better switched to the previous overlay. You can do it by duplicating the dialog, but restrict it with 'resolution', like for example, here:
https://github.com/scummvm/scummvm/blob/master/gui/themes/common/highres_layout.stx#L72

@bluegr
Copy link
Member

@bluegr bluegr commented Jun 11, 2022

PR #3982 has been opened as a possible solution to address the issue of the large spacing of the option dialogs in high resolutions

bluegr added a commit that referenced this issue Jun 12, 2022
PR #3976 changed the game and global options dialogs so that they are
inside the ScummVM main window, instead of the game list chooser.

We now limit the option dialog width and size, so that there isn't too
much spacing in large resolutions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants