-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
💄 Add splash animation within web index file #503
💄 Add splash animation within web index file #503
Conversation
I actually think that rocks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can dig it! I left a few comments I'd like to resolve before giving my ✅, but otherwise I think we can give this UI a try
apps/web/src/index.html
Outdated
try { | ||
const zustandState = JSON.parse( | ||
localStorage.getItem('stump-user-store') || '{"state": {}}', | ||
).state | ||
if (zustandState.userPreferences && zustandState.userPreferences.app_theme === 'dark') { | ||
document.querySelector('html').classList.add('dark') | ||
} | ||
} catch {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I originally wrote something like this for the desktop app back when there were only two themes. To make this work with the current system, it should be something like:
try { | |
const zustandState = JSON.parse( | |
localStorage.getItem('stump-user-store') || '{"state": {}}', | |
).state | |
if (zustandState.userPreferences && zustandState.userPreferences.app_theme === 'dark') { | |
document.querySelector('html').classList.add('dark') | |
} | |
} catch {} | |
try { | |
const zustandState = JSON.parse( | |
localStorage.getItem('stump-user-store') || '{"state": {}}', | |
).state | |
const appTheme = zustandState.userPreferences?.app_theme || '' | |
const alreadyHasTheme = | |
!appTheme || document.querySelector('html').classList.contains(appTheme) | |
if (!alreadyHasTheme) { | |
document.querySelector('html').classList.add(appTheme) | |
} | |
} catch {} |
html.dark { | ||
background-color: #161719; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure you already know where to find them since you pulled this value, but I'd want to make sure we cover each supported theme before merging this in.
A note for the future is that custom themes won't quite work using this method, since the CSS isn't loaded until the app mounts and this splash is present before that point
Co-authored-by: Aaron Leopold <36278431+aaronleopold@users.noreply.github.com>
Is this something we're interested in? Any concerns or improvements? we could support more themes if needed!
splash.mp4