Skip to content

[theme demo] Fix dropdown losing choice on solarized light / dark #7122

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

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

cben
Copy link
Contributor

@cben cben commented Mar 11, 2025

Mild bug in demo due to space handling.

Before: https://codemirror.net/5/demo/theme.html#solarized%20dark

  1. dropdown .value set to non-existant solarized%20dark => dropdown reset to blank.
  2. picking solarized dark / light from dropdown changes theme (correctly sets .cm-s-solarized .cm-s-dark classes),
    sets URL fragment to #solarized%20dark (implicitly encoded by browser?¹),
    which again triggers dropdown reset to blank (1).
  3. can't iterate all dropdown values by Down/Up arrow: once you reach solarized, (2) happens

After: https://raw.githack.com/cben/CodeMirror5/theme-demo-location-hash-DEBUG/demo/theme.html#solarized%20dark (from debug branch with extra console logging)

¹ ² This commit makes both setting and getting URL fragment reliable.

Choosing "solarized dark" correctly sets .cm-s-solarized .cm-s-dark
(as documented https://codemirror.net/doc/manual.html#option_theme).

It then sets URL fragment to #solarized%20dark,
which was looking for `solarized%20dark` in dropdown and failing.
This commit makes both setting and getting URL fragment reliable.
@marijnh
Copy link
Member

marijnh commented Mar 11, 2025

Thanks for fixing that!

@marijnh marijnh merged commit 064ea16 into codemirror:master Mar 11, 2025
1 check failed
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.

2 participants