Skip to content

fix(playground): preserve layout settings when switching themes#769

Merged
effie-ms merged 3 commits into
mainfrom
fix/playground-theme-switch-height-reset
Jun 15, 2026
Merged

fix(playground): preserve layout settings when switching themes#769
effie-ms merged 3 commits into
mainfrom
fix/playground-theme-switch-height-reset

Conversation

@effie-ms

@effie-ms effie-ms commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Which Linear task is linked to this PR?

EMB-424

Why was it implemented this way?

setConfigTheme replaced the entire config.theme with the new theme preset's clone, discarding the user's layout container settings (height, maxHeight, display).

The fix destructures only the layout-related container keys from the current config before applying the new theme, then merges them back on top of the new theme's container defaults. This preserves the user's height/layout choices while letting visual container properties (borderRadius, filter, etc.) update to match the selected theme.

Visual showcase (Screenshots or Videos)

Checklist before requesting a review

  • I have performed a self-review and testing of my code.
  • This pull request is focused and addresses a single problem.
  • If this PR modifies the Widget API or adds new features that require documentation, I have updated the documentation in the public-docs repository.

@changeset-bot

changeset-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 68e5d82

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vinzenzLIFI

Copy link
Copy Markdown
Contributor

minor find: for some reason, when i switch from default to jumper theme while having a max height set, there is a small gap at the bottom of the container. but if i switch from something else to jumper, then it looks clean only for the token container, the chain container still has the gap.

Screen.Recording.2026-06-08.at.14.58.30.mov

@vinzenzLIFI

Copy link
Copy Markdown
Contributor

should we persist dark mode in local storage? it changes back to light mode after refresh (not in scope of this PR)

@effie-ms

effie-ms commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

minor find: for some reason, when i switch from default to jumper theme while having a max height set, there is a small gap at the bottom of the container. but if i switch from something else to jumper, then it looks clean only for the token container, the chain container still has the gap.

What is happening there: the height of those long lists (e.g. tokens) is calculated dynamically (from the available space). And when we switch a theme, it is not recalculated. But the header's height seems to change because of borders.
Technically, this 2px gap is visible between a theme with the border around search input and a theme without the border. So I am thinking: if I recalculate the height on every theme switch (not to overcomplicate the parsing to find only borders) - it might not be optimal either. Restricting the header's height - also not really an option because we allow customizing the weight of borders.

I am thinking about leaving it as-is. This would be the problem only for the playground - because the integrators set the theme (borders) once and (typically) do not customize it - so they would not get affected.

@effie-ms

effie-ms commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

should we persist dark mode in local storage? it changes back to light mode after refresh (not in scope of this PR)

Good question. To be honest, I am not sure what behaviour is more expected by users (persistence or not) 😅
On reload - it persists, on reset or switching a theme - not sure.

@vinzenzLIFI

Copy link
Copy Markdown
Contributor

I agree with not overcomplicating a solution if it makes things worse.

Regarding the dark and light mode. When I set dark or light mode directly in the playground settings, it's persisted. however, when I set in the widget, it's not and will fall back to system. this is probably intended, but I wanted to mention anyway.

imho: on global reset everything should be reset back to how it was before. same should be the case for the custom theme settings. if we have a default, that must be recovered.

@github-actions

Copy link
Copy Markdown

E2E Playground results

passed  158 passed

Details

stats  158 tests across 10 suites
duration  1 minute, 58 seconds
commit  68e5d82

📥 Download full HTML report (open the run → Artifacts → playwright-report)

@effie-ms

Copy link
Copy Markdown
Contributor Author

Regarding the dark and light mode. When I set dark or light mode directly in the playground settings, it's persisted. however, when I set in the widget, it's not and will fall back to system. this is probably intended, but I wanted to mention anyway.

I checked v3 - we indeed did not persist it when toggling the dark/light from the widget. It did apply on the DOM level, but not to the local storage.
It's possible to enable though - I have made a separate PR: #778. But I think it would be worth discussing if we want it in general, before merging.

@vinzenzLIFI

Copy link
Copy Markdown
Contributor

sounds good to me. I approve this PR then and we can discuss further improvements in the new one you opened.

@tomiiide tomiiide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks goood!

@effie-ms effie-ms merged commit 1794df9 into main Jun 15, 2026
11 checks passed
@effie-ms effie-ms deleted the fix/playground-theme-switch-height-reset branch June 15, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants