-
Notifications
You must be signed in to change notification settings - Fork 916
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
Support overriding theme through URL search parameter #7791
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
feat: | ||
- Support overriding theme through URL search parameter ([#7791](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7791)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,16 +7,36 @@ try { | |
window.console.log(error); | ||
} | ||
|
||
var useBrowserColorScheme = window.localStorage.getItem('useBrowserColorScheme') === 'true' || false; | ||
/** @type {'browser' | 'light' | 'dark' | null} */ | ||
var urlThemeParam = null; | ||
try { | ||
urlThemeParam = new URLSearchParams(window.location.search).get('theme'); | ||
if (!urlThemeParam && window.location.hash.includes('?')) { | ||
urlThemeParam = new URLSearchParams(window.location.hash.split('?')[1]).get('theme'); | ||
Comment on lines
+14
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we specify urls params in the url fragment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll take it if it's in the hash. when hashrouter is used i think many parts of dashboards use hash for url params There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a future enhancement, we MIGHT be having an interceptor on the server side to handle language and theme changes among others. After intercepting, it will clean the URL and redirect to the destination without the lang and theme. If we look into the hash here, we cannot unlook at it later and that would make the transition challenging if not impossble without a breaking change. IMO, we should drop this piece and stick to the one above. |
||
} | ||
if (![null, 'dark', 'light', 'browser'].includes(urlThemeParam)) { | ||
throw new Error('theme must be "browser", "light", or "dark", received ' + urlThemeParam); | ||
urlThemeParam = null; | ||
} | ||
} catch (error) { | ||
window.console.warn('Failed to parse "theme" parameter in URL', error); | ||
} | ||
|
||
var useBrowserColorScheme = | ||
urlThemeParam === 'browser' || | ||
window.localStorage.getItem('useBrowserColorScheme') === 'true' || | ||
false; | ||
|
||
var browserDarkMode = uiSettings['theme:darkMode'] || {}; | ||
var browserDarkMode = urlThemeParam | ||
? { userValue: urlThemeParam === 'dark' } | ||
: uiSettings['theme:darkMode'] || {}; | ||
var browserThemeVersion = uiSettings['theme:version'] || {}; | ||
|
||
var rawDarkMode = typeof browserDarkMode.userValue !== 'boolean' ? {{configDarkMode}} : browserDarkMode.userValue; | ||
|
||
var rawThemeVersion = browserThemeVersion.userValue || '{{configThemeVersion}}' | ||
|
||
if ({{configEnableUserControl}}) { | ||
if (urlThemeParam || {{configEnableUserControl}}) { | ||
if (useBrowserColorScheme && window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches !== | ||
rawDarkMode) { | ||
uiSettings['theme:darkMode'] = window.matchMedia('(prefers-color-scheme: dark)').matches; | ||
|
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.
if this parameter only impacts dark/light mode, can we name it after that and not use
theme
?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.
yes it's like
'colorscheme'
vs'background'
, and "theme" only impacts background. i thought about something like darkMode, but it also needs to support "browser". do you have any recommendation?