Skip to content
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

fix(settings): Fix back button and side panel when opening settings #26702

Merged
merged 3 commits into from
Dec 6, 2024

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Dec 5, 2024

Problem

  1. Quoting @rafaeelaudibert:

    redirecting from project- to environment- [is] not replacing the URL, it's simply adding a new entry, so you need to click the back arrow twice

  2. Quoting @annikaschmid:

    The project settings button is not working anymore ["Configure" in session replay]

Both apply only behind the environments feature flag.

Changes

  1. The cause of redundant history items was an awkward interaction between urlToAction and actionToUrl in settingsSceneLogic:
    urlToAction called selectLevel/selectSection with the redirected path, which caused those actions' actionToUrl handlers to create new history items.
    Now selectLevel/selectSection replace the current history item, and we no longer use them in the settings scene.
    The settings scene uses URLs instead, which is overall better ergonomics (e.g. you can now use "Open in new tab" in settings).

  2. The redirect only applied in settingsSceneLogic, but we also now have analogous logic for side panel settings in the lower-level settingsLogic.

How did you test this code?

Clicked around in this case.

Copy link
Contributor

github-actions bot commented Dec 5, 2024

Size Change: 0 B

Total Size: 1.11 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.11 MB

compressed-size-action

Copy link
Member

@rafaeelaudibert rafaeelaudibert left a comment

Choose a reason for hiding this comment

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

I definitely need to study Kea better :). Even if I had found the right file I don't think I'd have been able to solve it.

Thanks for the fix! Tested locally and it looks good :)

@Twixes Twixes merged commit 7399d5a into master Dec 6, 2024
96 checks passed
@Twixes Twixes deleted the settings-replace branch December 6, 2024 08:41
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