Skip to content

fix+feat: show sidebar context-menu when it is "narrow"/add narrow mode#4062

Open
arikorn wants to merge 7 commits intobitfocus:mainfrom
arikorn:fix_show_context_menu_in_narrow_sidebar
Open

fix+feat: show sidebar context-menu when it is "narrow"/add narrow mode#4062
arikorn wants to merge 7 commits intobitfocus:mainfrom
arikorn:fix_show_context_menu_in_narrow_sidebar

Conversation

@arikorn
Copy link
Copy Markdown
Contributor

@arikorn arikorn commented Apr 1, 2026

The recently merged PR (#3995) to manage unfolding mode added a "temporary narrow" mode to the sidebar (user clicked in the narrow region and didn't leave the sidebar). This PR fixes it so the user can bring up the context-menu in that mode.

bonus: convert chained :not():not() to :not(a,b) lists.

Summary by CodeRabbit

  • Style
    • Refined sidebar hide/show behavior for narrow and unfoldable modes for more consistent label, toggler and context-menu visibility; adjusted group and active-item styling and scroller behavior for predictable collapsed/expanded presentation.
  • New Features
    • Added a persisted "narrow mode" option with a context-menu toggle to keep the sidebar folded; narrow mode now suppresses the bottom toggler, alters the hide-sidebar label when unfoldable, and is treated like mobile for context-menu behavior.

The recent PR to manage unfolding mode added a "temporary narrow" mode to the sidebar (user clicked in the narrow region and didn't leave the sidebar). This PR fixes it so the user can bring up the context-menu in that mode.

bonus: convert chained `:not():not()` to `not(a,b)` lists.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Sidebar SCSS selectors were reorganized to make .sidebar-narrow drive visibility rules; Sidebar component adds persisted narrowMode (sidebar_narrow_mode), renames the foldable storage key to sidebar_foldable, converts narrow into tempNarrow plus persisted narrowMode, and updates context-menu toggles/labels and bottom toggler visibility.

Changes

Cohort / File(s) Summary
Sidebar Styling Updates
webui/src/scss/_layout.scss
Reworked narrow/unfoldable selectors: replaced repeated :not(.show):not(:hover) with grouped :not(.show, :hover), made .sidebar-narrow (not hover) drive hide/visibility, restricted .context-menu hiding to unfoldable-only, refactored .nav-group scoping, and adjusted scroller/group-items visibility selectors.
Sidebar Component Behavior
webui/src/Layout/Sidebar.tsx
Persisted narrowMode (sidebar_narrow_mode) added; foldable key renamed to sidebar_foldable; ephemeral narrow becomes tempNarrow with `narrow={tempNarrow

Poem

A slimmer rail dons steadier code,
Keys renamed along the road,
Selectors tidy, toggles true,
Narrow holds — a faithful view,
Small tweaks, big welcome to the mode ✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main changes: fixing context-menu visibility in narrow mode and adding a persistent narrow mode feature, matching the PR's core objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

arikorn added 2 commits April 2, 2026 09:57
Previously, the left border only showed when the group was open, but this nicely highlights the group togglers, especially in the narrow sidebar.
Allow user to keep the sidebar narrow (applies to both regular and mobile mode).

This requires only very small tweaks to the existing code. Currently setting the mode is accessible only from the context-menu.
@arikorn arikorn changed the title fix: show sidebar context-menu when it is "narrow" fix+feat: show sidebar context-menu when it is "narrow"/add narrow mode Apr 2, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
webui/src/Layout/Sidebar.tsx (1)

263-274: Consider hiding the footer fold toggle while persistent narrow mode is active.

narrowMode wins over unfoldable here, so the footer toggle can still flip the stored fold/full-width mode with no visible change. That makes the control feel broken and can surprise users when they later turn off Keep Sidebar Folded.

Also applies to: 309-313


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1c45a556-a474-41f7-a515-e5b1f6b1e622

📥 Commits

Reviewing files that changed from the base of the PR and between 24e861a and fba358e.

📒 Files selected for processing (2)
  • webui/src/Layout/Sidebar.tsx
  • webui/src/scss/_layout.scss

- rename `narrow` to `tempNarrow` to make its intent clearer
- add function to set it false when entering narrowMode
- hide "sidebar-header-toggler" (i.e. foldable-mode toggler) when in narrow mode

While the function isn't strictly necessary, it keeps the behavior uniform.
@arikorn
Copy link
Copy Markdown
Contributor Author

arikorn commented Apr 2, 2026

The last (two) commits adds a feature allowing the user to keep the sidebar permanently narrow (including in mobile mode: when opened it opens in narrow mode). The feature is currently accessible only from the sidebar context menu. Most of the functionality was already built in, so this is a "trivial" upgrade (and it also tidies up a couple of lines of CSS that weren't needed).

@Julusian
Copy link
Copy Markdown
Member

Julusian commented Apr 4, 2026

Maybe when in this mode it should suppress the collections and connection variables pages, navigating those feels like too much guesswork to be useful.

Theres also some unexpected (to me) behaviour that when you click on a group, it expands/collapses that group as well as navigating to that page. so if I go variables->buttons->variables, the variables group is now collapsed

Also not related to this PR, but is it intentional that all expanded groups show highlighted the same as the active page?
image

@arikorn
Copy link
Copy Markdown
Contributor Author

arikorn commented Apr 4, 2026

The short answer is that these are all "pre-existing" to this PR, but worth re-examining. They are also all things I've touched in previous PRs. So...

Maybe when in this mode it should suppress the collections and connection variables pages, navigating those feels like too much guesswork to be useful.

Not guesswork, but you do have to hover for a second to see what the dot represents (i.e. tooltip tells you). A minor problem: currently the tooltip is on the icon itself, so you have to hover precisely... Several options come to mind (in order of my effort needed):

  • accept as-is (always the easiest 😀 )
  • encourage module makers to provide an icon in the manifest that could be used here (not sure if this would affect APIs)
  • use a bigger placeholder icon to increase the target area for tooltips
  • replace missing icons the first letter or two of the name (not so sure how easy this would be)
  • remove the module-variables from the sidebar in all modes. Personally I find that they add way too much clutter. Instead, we could make a single module-variables page similar to your "advanced settings" window in the launcher (two panels in around 1:3 proportion). The module variables already appear to be limited to 2/3 of the window-width so nothing is lost. (Or just make people use the current "index" page to get to module variables.)
  • make the tooltip-sensitive area larger w/o changing the icon. Unfortunately this is not trivial since the label component has to become aware of the sidebar mode (so it either needs context or passing narrowMode around).
  • in conjunction with the previous, replace the tooltip with CPopover, which can come up quickly and is more prominent. This, in fact, was my original intent and could be added later regardless. I didn't do it here because it too requires the "deep" change in the React code rather than just messing with CSS.
  • Regarding your suggestion: I could definitely do something clever but am bothered that it changes the sidebar between modes, i.e. things disappear, which might be confusing to the user. (And a shift in positions, too, when leaving this mode.)

Let me know your choice!

Theres also some unexpected (to me) behaviour that when you click on a group, it expands/collapses that group as well as navigating to that page. so if I go variables->buttons->variables, the variables group is now collapsed

Yeah, this has been around since I "fixed" the expand/collapse behavior on those buttons quite some time ago. Previously it would expand but not collapse until you clicked on the caret itself, which I found annoying and confusing. The problem, of course is that these buttons are serving double-duty of selecting the page and opening the group.

Not sure I understand your counterexample though. If you clicked variables to open the variable group, then went to buttons, the variable group is still open in the sidebar, so why would you need to click "variables" again except to close the group? But yes, it does close the group and go to the variables index page. (I suppose it would be nice to be able to click just the caret to open/close the group w/o navigation -- not sure if that's a tricky or simple change.)

Options:

  • restore the old behavior. The old behavior is slightly mitigated if one chooses the new accordion mode (auto-collapse) in the context-menu
  • don't allow "double-duty" togglers. I think the only real transgressor now is the surfaces toggler, which is problematic anyway since it is just a synonym for its first child.

Also not related to this PR, but is it intentional that all expanded groups show highlighted the same as the active page?

Yes, that was in one of my recent sidebar PRs. It think it helps highlight the opened group, but it's definitely not necessary if you don't like it. Just a note, going back to the surfaces toggler: it's what it does anyway "most" of the time, since it and its first child are always lit together! Anyway, I'm happy to turn this off in general (I could probably even turn it off for surfaces by making the "active" state non-fuzzy, though it might be confusing if the use selects the page then closes the group.)

So just let me know your choices on all of these!

@Julusian
Copy link
Copy Markdown
Member

Julusian commented Apr 4, 2026

If you clicked variables to open the variable group, then went to buttons, the variable group is still open in the sidebar, so why would you need to click "variables" again except to close the group?

Its another workflow to solve navigating these pages which all have the same icon. Instead of spending time trying to find the right circle, I would click on the variables page and use the list it presents to find the page I am after.

Yeah, this has been around since I "fixed" the expand/collapse behavior on those buttons quite some time ago. Previously it would expand but not collapse until you clicked on the caret itself, which I found annoying and confusing. The problem, of course is that these buttons are serving double-duty of selecting the page and opening the group.

actually that raises a bigger issue, in that there is no way to collapse a group other than to go to that page.. so once I leave the variables page the only way to collapse the group is to go back to it.

The long list of connections didnt bother me before because when I didnt want to see it I could collapse the variables section and ignore it

Hmm.. another bug here, when I refresh the page the 'current' group isn't expanded, making it hard to see where I currently am (without accidentally navigating away)

Regarding your suggestion: I could definitely do something clever but am bothered that it changes the sidebar between modes, i.e. things disappear, which might be confusing to the user.

I think that is fine for this mode, as this is an explicit opt in to keep it collapsed. A slight reshuffle of the content seems reasonable to ensure it makes sense for navigating.
I'm not entirely opposed to the option of showing

Instead, we could make a single module-variables page similar to your "advanced settings" window in the launcher

I'm not entirely opposed to this, I did have the same thought the other day, but I'm not 100% sure. That layout won't work for mobile either, but maybe thats fine?

I don't think I want a change like this before 4.3 is out though

I suppose it would be nice to be able to click just the caret to open/close the group w/o navigation -- not sure if that's a tricky or simple change.

Thats how it used to behave, it was a split button so separete hover styles. I dont remember if clicking the link portion would expand the group or not

restore the old behavior. The old behavior is slightly mitigated if one chooses the new accordion mode (auto-collapse) in the context-menu

From firing up 4.2, it used to behave that clicking the link toggled the group, but there was a separate button to trigger just the collapse/expand. I would say that clicking the link portion shouldnt ever collapse it (maybe unless it does no navigation?)

don't allow "double-duty" togglers. I think the only real transgressor now is the surfaces toggler, which is problematic anyway since it is just a synonym for its first child.

The problem with this is that it turns some navigations into having more clicks, especially if groups are auto-collapsing.

Yes, that was in one of my recent sidebar PRs. It think it helps highlight the opened group

I wouldn't mind a subtler style, but as shown in that screenshot the same colour causes it to very much drown out the active indication.
It used to be that only the expanded groups that were above the active link would show in this colour, not any that were expanded

arikorn added 2 commits April 5, 2026 00:30
- restore "split-button" functionality to group togglers
- - note that unlike 4.2 and earlier, all groups togglers are split, not just ones that do double-duty (i.e. nav). This maintains consistency in the UI and supports the next feature
- Open groups now only highlight the caret unless the nav-path is active.
- Clicking on the label of a nav-group will only close it if no navigation happens, i.e. either this is not a nav button or the user is already on that button.
- Clicking on the caret toggles the group without navigation
- In `narrowMode` a "popover" showing the full name -- or title if present -- appears to the right on hover anywhere in the button or on focus. (The default text for group toggles has the word "group" appended.)

Note: while this commit does not directly address the refresh-page issue (refresh-page doesn't preserve group open/closed state) it does allow you to open the group w/o navigating in all modes but narrow.

Technical note: I added a new context component to support some of these features. Two alternatives are: (1) use the SidebarStateContext, which would be a little more fiddly and may create extra renders or (2) pass the arg down to the relevant menu-items, which requires changing all menu-item definitions as well as the generic `CNavGroup` definition (which required modification anyway, so maybe not a big deal?) -- may or may not be a better option.
@arikorn
Copy link
Copy Markdown
Contributor Author

arikorn commented Apr 6, 2026

The latest commit address (almost) all of your issues, @Julusian. To copy from and add to the commit message:

  • "split-button" functionality on group togglers is restored
    • note that unlike 4.2 and earlier, all groups togglers are split, not just ones that do double-duty (i.e. nav). This maintains consistency in the UI and supports the next feature
  • Expanded groups now only highlight the caret unless the nav-path is active, see image below (i.e. active and expanded states are independent).
  • Clicking on the label of a nav-group will only close the group if no navigation happens, i.e. either this is not a nav button or the user is already on that button.
  • Clicking on the caret toggles the group without navigation
  • In narrowMode a "popover" showing the full name -- or title if present -- appears to the right on hover anywhere in the button or on focus, see image below. (The default text for group toggles has the word "group" appended.)
  • improve horizontal centering of icons in narrow mode
  • remove group-item for configured surfaces since it's just a duplicate of what the toggler does.

Hmm.. another bug here, when I refresh the page the 'current' group isn't expanded, making it hard to see where I currently am (without accidentally navigating away)

While this commit does not directly address the refresh-page issue (refresh-page doesn't preserve group open/closed state) it does allow you to open the group w/o navigating in all modes but narrow...

My opinion here: Losing state on refresh: (a) it's a feature not a bug -- allows you to reset a (perceived) mess (b) although a simple way to "keep" state would be to open the group if something in it is selected, it would probably then prevent closing the group if the user wanted to close it.

The problem with [not allowing double-duty togglers] is that it turns some navigations into having more clicks, especially if groups are auto-collapsing.

Understood. In this commit I removed the surfaces subpage, since it's just noise and ever-so-slightly confusing.

I just realized a minor bug: "surfaces" isn't active when "remote surfaces" is selected. The problem is that getting the "only close if no nav happened" to work requires matching the exact page, so for it to work with surfaces, the group toggler has to be set to /surfaces/configured (since surface just forwards there), but remove surfaces route is not nested under configured... So the solutions could be to

  • reconfigure the routes, probably merging configured into /surfaces, (which probably makes sense anyway) or
  • navigate to /surfaces and have the surfaces group toggle behave differently than all others (not recommended!)
  • live with the current behavior (it's pretty subtle)

Instead, we could make a single module-variables page similar to your "advanced settings" window in the launcher

I'm not entirely opposed to [removing connection-variables icons from the sidebar], ... I don't think I want a change like this before 4.3 is out though

Agreed, but anticipating such a change bolsters the case, in this PR, for not hiding items that don't have an icon.


Technical note: I added a new context component to support some of the new features. Two alternatives are:

  1. use the SidebarStateContext, which would be a little more fiddly and may create extra renders or
  2. pass the arg down to the relevant menu-items, which requires changing all menu-item definitions as well as the generic CNavGroup definition (which required modification anyway, so maybe not a big deal?) -- may or may not be a better option.

Let me know if you have an opinion. Otherwise, I'm happy with the current way despite a bit of white-space noise it introduces (i.e indenting the entire return group of MySidebar).


Open group vs. active group highlighting Popover in narrow mode (applies to all buttons, not just ones missing an icon)
image image

@arikorn
Copy link
Copy Markdown
Contributor Author

arikorn commented Apr 7, 2026

I wrote:

reconfigure the routes, probably merging configured into /surfaces, (which probably makes sense anyway) or

I think this needs to be done, regardless. The current structure doesn't really reflect the page contents properly (the page is more than just the configured surfaces). I'll post another PR.

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