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

Mainmenu: Avoid the header being displayed behind the formspec #13924

Merged
merged 2 commits into from Nov 25, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Oct 26, 2023

Currently, the mainmenu header image is usually displayed behind the mainmenu formspec on Android. This results in three problems:

  • The header is hard/impossible to read.
  • The part of the formspec that is in front of the header is hard to read.
  • It looks ugly.

This PR keeps the current header placement code, but adds additional code to make sure the header doesn't end up behind the formspec. I don't claim that it's beautiful now, but I think it's better than before.

Here's a before/after comparison on Android:
screenshot before this PR screenshot after this PR

On desktop, it looks the same as before.

To do

This PR is a Ready for Review.

A future PR could move the mainmenu formspec down a bit, giving more space to the header.

How to test

Start Minetest on your Android device. Verify that the mainmenu header isn't displayed behind the mainmenu formspec.

Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

This PR looks good to me.


My tests (on Samsung Galaxy A72):

GUI scaling = 0.5

Main Menu with GUI scaling 0.5

GUI scaling = 0.75

Main Menu with GUI scaling 0.75

GUI scaling = 1.0

Main Menu with GUI scaling 1.0

GUI scaling = 1.1

Main Menu with GUI scaling 1.1

The header image is still shown until GUI scaling 1.19.

GUI scaling = 1.2

Main Menu with GUI scaling 1.2

@grorp
Copy link
Member Author

grorp commented Oct 28, 2023

Apparently the header was usually slightly behind the formspec on desktop as well, and this PR causes the header to move a few pixels when you open a dialog. I'll try to improve this.

@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 28, 2023
@srifqi
Copy link
Member

srifqi commented Oct 30, 2023

By the way, this also works for non-Android version. I tested this PR on Windows:

Minetest-main-menu-header-bigger-gui-scale

@grorp grorp changed the title Mainmenu: Fix the header being displayed behind the formspec on Android Mainmenu: Avoid the header being displayed behind the formspec Oct 30, 2023
@grorp grorp removed Android Action / change needed Code still needs changes (PR) / more information requested (Issues) labels Oct 30, 2023
@grorp
Copy link
Member Author

grorp commented Oct 30, 2023

The header moves back and forth less again on desktop since my latest commit.

But if the header usually ends up (slightly) behind the formspec even on desktop, maybe we should consider adjusting the original header placement code instead of just correcting its output.

Anyway, the approach taken by this PR is probably fine as a simple fix for the Android issue.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Works.

@SmallJoker SmallJoker merged commit 4255ac3 into minetest:master Nov 25, 2023
13 checks passed
cx384 pushed a commit to cx384/minetest that referenced this pull request Dec 9, 2023
…est#13924)

This change keeps the current header placement code, but adds additional code to make sure the header doesn't end up behind the formspec.
@grorp grorp deleted the fix-header-behind-menu branch December 18, 2023 16:40
kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023
…est#13924)

This change keeps the current header placement code, but adds additional code to make sure the header doesn't end up behind the formspec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants