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

Game.styl: remove unnecessary 8px right margin on "center-col", so board can get larger #2585

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

pdg137
Copy link
Contributor

@pdg137 pdg137 commented Feb 18, 2024

I did this before I noticed @benjaminpjones's #2582. My main goal was to fix mobile, but still I think we can get rid of the 8px margin under all circumstances. The right-col already has its own left margin so I don't see why we should also have a right margin on center-col.

Before:

image

After:

image

Without the highlighting, so you can see it's not ridiculous:

image

A larger window size showing the closest the board ever gets to the stuff on the right:

image

Copy link

github-actions bot commented Feb 18, 2024

Uffizzi Preview deployment-46769 was deleted.

@anoek
Copy link
Member

anoek commented Feb 18, 2024

Does the "dancing screen" aka flickering resize issue happen at all as you slowly resize the window at any point?

I am not sure why the margin fixed that issue, but the margin was introduced to resolve that issue and it was reported that it fixed it at the time.

@pdg137
Copy link
Contributor Author

pdg137 commented Feb 18, 2024

Okay, I checked the following:

I did a git bisect checking for versions where the 8px margin is required and found e970a2c which is clearly what fixed the problem for good, though it's kind of sad that we needed to do it like that.

More generally, I think removing things that change at particular sizes - like I'm doing in this PR - is going to help the layout be more stable and prevent problems like this from popping up mysteriously under certain conditions.

@anoek
Copy link
Member

anoek commented Feb 18, 2024

Very cool, thanks for doing that research, glad to be rid of the hack!

@anoek anoek merged commit 69aeed5 into online-go:devel Feb 18, 2024
4 checks passed
@benjaminpjones
Copy link
Contributor

Awesome! Thanks for looking into that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants