feat: added theme switcher with match-system option #1273
Conversation
kirill-dev-pro
commented
Nov 25, 2024
kirill-dev-pro
commented
kirill-dev-pro
force-pushed
the
feature/auto-theme
branch
from
0cb9d71
to
17a861d
Compare
|
привет! а мы не можем то же самое на Vue сделать? Я не помню, чтобы мы WebComponents использовали во фронтенде и если это первый случай, я бы предпочел не размывать тех.стек и сделать все на вью. |
| @@ -9,12 +9,6 @@ | |||
| {% block meta %} | |||
| {% include "common/meta.html" %} | |||
| {% endblock %} | |||
| <script> | |||
| const theme = localStorage.getItem('theme'); | |||
There was a problem hiding this comment.
В некоторых местах далее проверяется typeof localStorage !== 'undefined' перед юзанием LS. Предлагаю что тут, что далее юзать nullsafe, т.е. вместо проверок и дерева из if, просто юзать
const theme = localStorage?.getItem('theme') ?? 'auto'
| theme === 'auto' || theme === 'dark' || theme === 'light' ? theme : 'auto'; | ||
|
|
||
| const loadTheme = () => | ||
| parseTheme(typeof localStorage !== 'undefined' && localStorage.getItem(storageKey)); |
There was a problem hiding this comment.
Выше уже писал, предлагаю юзать nullsafe
parseTheme(localStorage?.getItem(storageKey));
| function storeTheme(theme) { | ||
| if (typeof localStorage !== 'undefined') { | ||
| localStorage.setItem(storageKey, theme === 'light' || theme === 'dark' ? theme : ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
зачем это в глобальной видимости?
| matchMedia('(prefers-color-scheme: light)').matches ? 'light' : 'dark'; | ||
|
|
||
| matchMedia(`(prefers-color-scheme: light)`).addEventListener('change', () => { | ||
| if (loadTheme() === 'auto') onThemeChange('auto'); |
There was a problem hiding this comment.
onThemeChange метод находится в классе StarlightThemeSelect, это просто не работает и вызывает ошибку.
| constructor() { | ||
| const theme = this.storedTheme || | ||
| (window.matchMedia('(prefers-color-scheme: light)').matches ? 'light' : 'dark'); | ||
| document.documentElement.dataset.theme = theme === 'light' ? 'light' : 'dark'; |
There was a problem hiding this comment.
dataset оперирует data- аттрибутами, в данном случае как я понял предполагается поменять theme аттрибут, а не data-theme.
|
|
||
| updatePickers(theme = this.storedTheme || 'auto') { | ||
| document.querySelectorAll('starlight-theme-select').forEach((picker) => { | ||
| console.log('picker', picker); |
There was a problem hiding this comment.
Надо бы кильнуть, пакер не убирает логи из продакшен-бандла
| constructor() { | ||
| super(); | ||
| this.onThemeChange(loadTheme()); | ||
| this.querySelector('select')?.addEventListener('change', (e) => { |
| super(); | ||
| this.onThemeChange(loadTheme()); | ||
| this.querySelector('select')?.addEventListener('change', (e) => { | ||
| if (e.currentTarget instanceof HTMLSelectElement) { |
There was a problem hiding this comment.
событие change, прослушиваемое на select теге всегда же будет возвращать этот самый селект как target, зачем проверять?
| const tmpl = document.querySelector(`#theme-icons`); | ||
| const newIcon = tmpl && tmpl.content.querySelector('.' + theme); | ||
| if (newIcon) { | ||
| const oldIcon = picker.querySelector('starlight-theme-select i.label-icon'); | ||
| if (oldIcon) { | ||
| oldIcon.replaceChildren(...newIcon.cloneNode(true).childNodes); | ||
| } | ||
| } |
