Skip to content

Fix: Sidebar group should toggle open/close on click (#5263)#5267

Open
Harshit-0413 wants to merge 6 commits intocodecentric:masterfrom
Harshit-0413:fix-5147
Open

Fix: Sidebar group should toggle open/close on click (#5263)#5267
Harshit-0413 wants to merge 6 commits intocodecentric:masterfrom
Harshit-0413:fix-5147

Conversation

@Harshit-0413
Copy link
Copy Markdown
Contributor

Problem

In the instance sidebar, once a group (e.g., "Web") is expanded, clicking it again does not collapse it.

Root Cause

The toggle logic did not correctly reset the open group state when the same group was clicked again.

Solution

Updated the toggle logic to properly toggle between open and closed states.

Notes

  • Fixed ESLint issues
  • Minimal and focused change

@Harshit-0413 Harshit-0413 requested a review from a team as a code owner April 16, 2026 14:38
</ul>
</li>

<template v-if="customLinksFromMetadata?.length > 0">
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.

Why has this been deleted?

I assume - @SteKoe please correct me if I'm wrong - that this part was implementing #4700

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @cdprete, that deletion was accidental. I've reverted it in the latest commit , the customLinksFromMetadata template block is restored. Sorry for the noise!

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.

No worries.
I just wanted to prevent an accidental dropping of the feature.

Copy link
Copy Markdown
Contributor

@cdprete cdprete left a comment

Choose a reason for hiding this comment

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

To my understanding, too many things have been changed/removed which are unrelated to the bug itself.

Moreover, if I'm right about https://github.com/codecentric/spring-boot-admin/pull/5267/changes#r3095328671, @SteKoe already knows I would open a regression bug if that feature disappears with no real reason ;)

@Harshit-0413 Harshit-0413 force-pushed the fix-5147 branch 2 times, most recently from e99ba38 to f934555 Compare April 16, 2026 18:37
<!-- Le subnav -->
<ul
v-if="hasMultipleViews(group) && isActiveGroup(group)"
v-if="hasMultipleViews(group) && (openGroup === group.id || isActiveGroup(group))"
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.

Probably openGroup === group.id could potentially be moved inside isActiveGroup as well.

Also, I think, the condition should a && and not a ||.

To my understanding of your changes, when a group gets closed then you set the openGroup to null.

The above condition, unless I'm missing something, would still evaluate to true because isActiveGroup(group) would still return true as before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved the toggle logic into a separate isGroupOpen function to keep isActiveGroup clean and unchanged. The subnav now uses isGroupOpen for visibility. Thanks for the review!

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.

Is it pushed?
I see no changes with the previous review.

@SteKoe
Copy link
Copy Markdown
Contributor

SteKoe commented Apr 17, 2026

Please rebase your branch correctly as it contains backend changes that are not related to frontend (obviously).

@cdprete
Copy link
Copy Markdown
Contributor

cdprete commented Apr 17, 2026

Please rebase your branch correctly as it contains backend changes that are not related to frontend (obviously).

Yeah, I wanted to suggest the same, but then I saw there were merge commits added by you and @ulischulte, so I was not sure anymore who the culprit was. 😅

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.

4 participants