fix(desktop): sidebar toggle button hidden after collapse#3141
Closed
HUQIANTAO wants to merge 1 commit into
Closed
fix(desktop): sidebar toggle button hidden after collapse#3141HUQIANTAO wants to merge 1 commit into
HUQIANTAO wants to merge 1 commit into
Conversation
Regression of esengine#2987: the rule .sidebar--collapsed .sidebar__brand > span { display: none } is too broad. The Tooltip component wraps its child in a <span> trigger (Tooltip.tsx: `<span ref={setTriggerRef} {...triggerProps}>{children}</span>`), and the toggle button lives inside that span. So once the rail collapsed, the > span rule also hid the Tooltip wrapper, the toggle button disappeared with it, and there was no UI left to expand the sidebar back. Only a full app restart (or `localStorage.removeItem` of the reasonix.sidebar.collapsed key) could recover it. The original esengine#2987 fix introduced a dedicated .sidebar__brand-name class and narrowed the selector, but the change was reverted at some point (the class is gone and the > span rule is back). Re-apply the fix, and add a belt-and-suspenders override `.sidebar--collapsed .sidebar__brand .tooltip-trigger { display: inline-flex }` so a future regression in the brand-name selector cannot lock the rail closed again. Comments on both sides point at esengine#2987 so the next person to touch this code sees why the indirection exists.
Owner
|
Thanks for chasing this one down — the diagnosis (the broad |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The left rail's chevron toggle disappears the moment the rail collapses, leaving the user with no way to re-expand the sidebar. Only a full app restart (or
localStorage.removeItem('reasonix.sidebar.collapsed')) could recover it.Root cause
The collapsed-rail CSS rule
is too broad. The brand block is
and the
Tooltipcomponent wraps its child in a<span ref={setTriggerRef} {...triggerProps}>{children}</span>(seeTooltip.tsx). So the> spanselector matched two spans — the brand text span (intended) and the Tooltip wrapper around the toggle button (unintended). Once the rail collapsed, the Tooltip wrapper gotdisplay: none, taking the toggle button with it. The rail was then a 68 px dead zone with no way out.Regression
PR #2987 (
9d3a7e7b) fixed this exact bug by adding a.sidebar__brand-nameclass and narrowing the selector to.sidebar--collapsed .sidebar__brand-name. That fix was reverted at some point in the main-v2 history (the class is gone, the broad> spanrule is back), so the bug resurfaced.Fix
Re-apply the original fix: add
sidebar__brand-nameto the brand text span inApp.tsxand narrow the CSS selector.Add a belt-and-suspenders defensive override
so a future revert of the brand-name class (or a similar regression) cannot lock the rail closed again. The Tooltip wrapper is now explicitly visible regardless of what other rules touch the brand's spans.
Comments on both sides of the change explain why the class indirection exists and link to PR fix(desktop): sidebar toggle button hidden after collapse #2987, so the next person who looks at this code sees the trap.
Files
desktop/frontend/src/App.tsx<span>Reasonix</span>→<span className="sidebar__brand-name">Reasonix</span>plus an inline comment.desktop/frontend/src/styles.css.sidebar--collapsed .sidebar__brand > spanwith the targeted.sidebar--collapsed .sidebar__brand-name, add the defensive.tooltip-triggeroverride, and document why.Diff stat
Verification
pnpm typecheck→ clean.pnpm build→ clean (✓ built in 1.72s).PanelLeftOpenicon is still visible at the top of the rail, click it, and confirm the rail re-expands. Repeat in both themes and on bothdarwinand the non-darwin padding variant.