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: dark mode logo now shows when specified #6077

Merged
merged 3 commits into from
May 16, 2023
Merged

Conversation

nikk15
Copy link
Contributor

@nikk15 nikk15 commented May 15, 2023

  • Dark mode logo was not showing for some accounts since the HMC logo fix. Discovered that users had to manually add "darkModeDefault: true" to their theme in the admin panel. This is not obvious, so I added a check for the inclusion of "dark-mode" in the theme id.
  • Minor fix for the import of svgs in TS included in this PR
  • This PR also changes the local env TIER variable to paid tier "p1".

NOTE: I'm not sure I agree with the darkModeDefault property - when we tidy up custom theming, I'll look into alternatives.

@@ -80,7 +80,7 @@ module.exports = (env, argv) => {
RETICULUM_SERVER: "hubs.local:4000",
POSTGREST_SERVER: "",
ITA_SERVER: "turkey",
TIER: "p0"
TIER: "p1"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is my own curiosity why do we need this one?

Copy link
Contributor Author

@nikk15 nikk15 May 15, 2023

Choose a reason for hiding this comment

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

Especially with the incoming work around theme customization, we use the admin panel theme tab a lot more now. It made sense to me to update this rather than manually do it each time.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah cool!

@@ -57,7 +57,7 @@ export function useTheme(themeId) {

function getAppLogo(darkMode) {
const theme = getCurrentTheme();
const shouldUseDarkLogo = theme ? theme.darkModeDefault : darkMode;
const shouldUseDarkLogo = theme ? (theme.darkModeDefault || theme.id.includes("dark-mode")) : darkMode;
Copy link
Contributor

Choose a reason for hiding this comment

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

yea I agree with your comment - would be good to have one source of truth for if it is dark mode or not.

@nikk15 nikk15 temporarily deployed to hc-bio May 15, 2023 18:47 — with GitHub Actions Inactive
@nikk15 nikk15 temporarily deployed to smoke May 15, 2023 18:47 — with GitHub Actions Inactive
Copy link
Contributor

@nickgrato nickgrato left a comment

Choose a reason for hiding this comment

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

lgtm

@nikk15 nikk15 merged commit 109f760 into master May 16, 2023
12 checks passed
@nikk15 nikk15 deleted the dark-mode-theme-fix branch May 16, 2023 14:11
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.

None yet

2 participants