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

Fix platform flash on download page #35

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ZirixCZ
Copy link

@ZirixCZ ZirixCZ commented Dec 15, 2023

It is arguably prettier for the platform info to show up with a nice transition than changing from one to another. We could theoretically make a not-loaded state that would be sent from the server so there is not an empty space there until it loads, then transition nicely to the actual platform nicely. What is the opinion of you all on this?

2023-12-15.23-53-51.mp4

@ZirixCZ
Copy link
Author

ZirixCZ commented Dec 15, 2023

The problem with showing an empty container is that each of the platforms have different height - if we were to make it an empty container we would have to make it clear that it is loading and then transition to the actual size. I'll try to cook up something nice.

@Vendicated
Copy link
Member

sorry i don't really understand what this is achieving? the original looks way better to me than yours

@ZirixCZ
Copy link
Author

ZirixCZ commented Dec 16, 2023

2023-12-16.01-44-33.mp4

This PR is fixing the platform flash that happens everytime you're not on windows. The part is causing trouble that we don't know the height that the windows platform will have on the server side (to make it the default one).

This kinda is the goal, but without hardcoding the height:

2023-12-16.01-48-20.mp4

@ZirixCZ
Copy link
Author

ZirixCZ commented Dec 16, 2023

2023-12-16.02-58-26.mp4

The commit at e233331 resolves the issue by saving the platform information in a cookie. This prevents the page from flashing. However, a user visiting the download page as their first page for the first time will still see an empty box until the page fully loads, which is unavoidable.

@Vendicated
Copy link
Member

i don't think this is desirable. the current behaviour seems better to me, and it's not really a big deal

you also seem to be using cookies inside a prerendered route. that won't work

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.

2 participants