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

SettingsSideBar: Fix selection of the correct row #336

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

leolost2605
Copy link
Member

There was a mix up happening with settingspage title and the name of the visible child in the stack. This would cause e.g. when opening a plug for the first time via a search result the wrong result to show up. Also in further searches the wrong row would be selected. All of this is fixed now.

@leolost2605 leolost2605 requested a review from a team January 8, 2025 16:58
@leolost2605 leolost2605 force-pushed the leolost/fix-row-selection branch 2 times, most recently from 34ccaef to b95a86c Compare January 8, 2025 17:04
@leolost2605 leolost2605 force-pushed the leolost/fix-row-selection branch from b95a86c to a44823c Compare January 8, 2025 17:05
Copy link
Contributor

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

The changes make sense and everything related seems to work including the described issue.

@jeremypw jeremypw merged commit 054ab20 into main Feb 5, 2025
4 checks passed
@jeremypw jeremypw deleted the leolost/fix-row-selection branch February 5, 2025 13:13
@danirabbit
Copy link
Member

This is an API break and doesn't work for stack pages that don't have a name. For example this breaks sharing settings

@leolost2605
Copy link
Member Author

leolost2605 commented Feb 6, 2025

Yeah I probably should've put that in the PR description.
Though I'd still argue that with this being private API we should go somewhere along this route with the appropriate PRs in the plugs.

@danirabbit
Copy link
Member

Yeah I'm not opposed to the change itself, it just is causing breakage right now. So we should add a new API and deprecate the old one

lenemter pushed a commit that referenced this pull request Feb 7, 2025
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.

3 participants