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

feat(ui): Add dark app loading theme #81611

Merged
merged 9 commits into from
Dec 10, 2024
Merged

Conversation

scttcper
Copy link
Member

@scttcper scttcper commented Dec 3, 2024

uses react_context to add theme-user_option_theme to the body's class to avoid screen flashing before the app loads

inverted the svg which is inside the img tag which makes it harder to style. made it slightly opaque since the black/white was a bit loud.

image

fixes #81555
related #81654

gets the user's config from `window.__initialData` and attempts to style the initial ui by adding a class to the body
@github-actions github-actions bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Dec 3, 2024
Copy link
Contributor

github-actions bot commented Dec 3, 2024

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

Copy link

codecov bot commented Dec 3, 2024

Bundle Report

Changes will increase total bundle size by 1.09kB (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
app-webpack-bundle-array-push 32.01MB 1.09kB (0.0%) ⬆️

src/sentry/templates/sentry/base-react.html Outdated Show resolved Hide resolved
.theme-dark {
.loading {
.loading-indicator {
// surface200
Copy link
Member

Choose a reason for hiding this comment

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

thank you for leaving the comments, makes finding future references much easier!

BYK added a commit that referenced this pull request Dec 4, 2024
This patch refactors a 10-year-old hack where we used a template tag to produce the initial data JSON, making the contents of that blob unavailable to any Django template. With this, all views extending from base gets a `react_config` context object so we can do things like sniffing user's language or theme preferences if available and modify the HTML output based on that.

Related #81611
BYK added a commit that referenced this pull request Dec 6, 2024
This patch refactors a 10-year-old hack where we used a template tag to
produce the initial data JSON, making the contents of that blob
unavailable to any Django template. With this, all views extending from
base gets a `react_config` context object so we can do things like
sniffing user's language or theme preferences if available and modify
the HTML output based on that.

Related #81611
@scttcper scttcper marked this pull request as ready for review December 7, 2024 00:54
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #81611   +/-   ##
=======================================
  Coverage   80.34%   80.34%           
=======================================
  Files        7272     7272           
  Lines      321262   321266    +4     
  Branches    20947    20947           
=======================================
+ Hits       258123   258127    +4     
  Misses      62736    62736           
  Partials      403      403           

@@ -18,3 +18,5 @@
</div>
</div>
{% endblock %}

{% block wrapperclass %}{{ user_theme }}{% endblock %}
Copy link
Member

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Uh-mazing! 🌟

@JonasBa
Copy link
Member

JonasBa commented Dec 10, 2024

Thank you @BYK and @scttcper, this has bugged me for quite some time ❤️

mifu67 pushed a commit that referenced this pull request Dec 10, 2024
This patch refactors a 10-year-old hack where we used a template tag to
produce the initial data JSON, making the contents of that blob
unavailable to any Django template. With this, all views extending from
base gets a `react_config` context object so we can do things like
sniffing user's language or theme preferences if available and modify
the HTML output based on that.

Related #81611
@scttcper scttcper merged commit c3b94ea into master Dec 10, 2024
52 checks passed
@scttcper scttcper deleted the scttcper/dark-app-loading branch December 10, 2024 21:00
@github-actions github-actions bot locked and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

White flash on refresh when using Dark Theme
3 participants