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

New loading screen design #13

Merged
merged 2 commits into from
Aug 6, 2022
Merged

New loading screen design #13

merged 2 commits into from
Aug 6, 2022

Conversation

Segergren
Copy link
Contributor

Here is a Preview
Works good for now but needs improvement.

Copy link
Owner

@lulzsun lulzsun left a comment

Choose a reason for hiding this comment

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

It looks 10x better than what we have right now, but I think this implementation needs more changes before it is ready to merge.

Here are some things I would suggest doing:

  • Instead of creating another pictureBox for this, reuse pictureBox1
    • the reason you want to use pictureBox1 is because there is some dynamic placement for this loading gif. see here (RefreshLoader())
      • possibly do something similar to the label?
  • Have the gif be transparent if possible, that way we can maybe theme for white in the future without having any issues
  • remove references to old loading gif

@Segergren
Copy link
Contributor Author

Reused the pictureBox1 and removed the old references.
Transparent GIFs doesn't work well with background color when I tried. And I would also use darker color for the second (white) color in the loading gif.

@Segergren Segergren requested a review from lulzsun August 6, 2022 17:41
Copy link
Owner

@lulzsun lulzsun left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@lulzsun lulzsun merged commit 2a12da9 into lulzsun:main Aug 6, 2022
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