UI coherence with the upcoming new website#10
Conversation
|
@lucasbalieiro can you take a look at this please? |
| useEffect(() => { | ||
| const observer = new MutationObserver(() => { | ||
| const isDark = document.documentElement.classList.contains('dark'); | ||
| setThemeState(isDark ? 'dark' : 'light'); | ||
| }); | ||
| observer.observe(document.documentElement, { attributes: true, attributeFilter: ['class'] }); | ||
| return () => observer.disconnect(); | ||
| }, []); |
There was a problem hiding this comment.
TODO: check if this MutationOberver is really necessary. Is a good strategy, but might introduce a loop.
Where the user clicks the SetTheme button -> React changes the DOM -> MutationObserver capture the DOM changes and trigger React again, which triggers the DOM again and so on.
Have to check the stratumprotocol.org how it is handling the transition between themes. Because I think it is using localstorage also. So, we problably don't need the MutationObserver
There was a problem hiding this comment.
from what I understand from both projects’ code, this MutationObserver doesn’t seem necessary. I might be missing some context, but based on what I could understand and test, we should be able to remove this useEffect entirely.
There was a problem hiding this comment.
Just one thing wrt theme selector, unrelated to this,I focused on is that it's coherent with website, so for example if user's settings are dark, website is dark so is wizard, otherwise things look messy if wizard goes into light and website was in dark mode, so just ensure there's consistency in the approach.
| } | ||
|
|
||
| // 3. Fall back to system preference | ||
| if (typeof window !== 'undefined') { |
There was a problem hiding this comment.
| if (typeof window !== 'undefined') { | |
| typeof window !== 'undefined' && 'matchMedia' in window { |
There was a problem hiding this comment.
this will force the matchMedia to be used only if it is actually implemented by the client side
|
@GitGab19, I reviewed the changes. Most of them adjust Tailwind styling, so I focused mainly on the new theme selector feature. The rest appears to be CSS updates. I noticed some borders seem to be missing, but I’m not sure if that’s intentional. For example, I see this when I point the stratumprotocol.org project to this wizard with the changes from this PR: 
|
|
@lucasbalieiro when testing within against a website, best is to do it in context of stratum-mining/stratumprotocol.org#300, as explained here, to me it seems you tested with current stratumprotocol.org. Locally I don't see the issue with the borders. |
|
About the theme selector, feel free to take it over guys you know this shit way better than I do :) |
|
I ran this locally and I have seen no issues with the borders you mentioned @lucasbalieiro. Given that both the new website and |
We only need to keep the theme contract in sync, not the entire configs. Both projects must define the same semantic color keys in |
3 participants
Stylistic changes to ensure wizard has the same style as the upcoming website stratum-mining/stratumprotocol.org#300
Screen.Recording.2026-02-06.at.17.51.24.mov