You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: don't auto-open right sidebar by default (#1923)
* fix: don't auto-open right sidebar by default
The navigation (course outline) sidebar should be the default; PR #1713
removed the alwaysOpenAuxiliarySidebar waffle flag guard but kept the
auto-open branches in place, effectively hardcoding the flag to true.
- useInitialSidebar defaults to COURSE_OUTLINE when nothing is stored,
and honors a stored RIGHT panel preference even before async widget
data has loaded (avoids a transient flicker that downstream effects
would otherwise write back to storage).
- Add a session-scoped "sidebar closed by user" flag in sessionStorage,
parallel to the existing outline-collapsed flag. toggleSidebar sets it
when the resulting state is null, clears it on open.
- useResponsiveBehavior and useUnitShiftBehavior respect the flag so a
user-closed state survives unit navigation and full-page remounts
within the same tab session.
Refs: openedx/wg-build-test-release#574
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: drop OUTLINE_SIDEBAR_HIDDEN in favor of SIDEBAR_CLOSED_BY_USER
The OUTLINE_SIDEBAR_HIDDEN sessionStorage flag was set only by the
course-outline collapse trigger, which already calls toggleSidebar(null) →
setSidebarClosedByUser(true). Every reader of isOutlineSidebarCollapsed
runs inside a path where closed-by-user is already false (either explicit
top-level checks or transitive via useInitialSidebar's null return), so
outline-hidden=true was never observably distinct from closed-by-user=true.
- Remove the constant, the two storage helpers, and all call sites in
useInitialSidebar / useUnitShiftBehavior / useResponsiveBehavior /
useSidebarSync / course-outline/hooks.js.
- useSidebarSync swaps the flag in-place rather than adding a top-level
early-return, so the corrective setCurrentSidebar(null) branches still
fire if currentSidebar is somehow stale while closed-by-user is true.
- useInitialSidebar: also refine the stored-RIGHT-panel block so that when
widget data has loaded and the stored preference is no longer in the
registry, we fall back to firstAvailable instead of returning a stored
ID nothing can render.
- Clarify the isInitiallySidebarOpen JSDoc to spell out the viewport/URL
precondition.
- Update ARCHITECTURE.md and the course-outline README to reference the
unified flag.
Refs: openedx/wg-build-test-release#574
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: keep current sidebar open across unit navigation
useUnitShiftBehavior was actively switching state on unit nav in ways that
contradict the spec in wg-build-test-release#574:
- CASE 1 (COURSE_OUTLINE open + RIGHT panel available) was switching to the
RIGHT panel, which conflicts with expected behavior #2 ("nav stays open
on advance"). The behavior looked correct in the browser only because
useSidebarSync immediately reverted the switch via stale-memo cancellation —
two unnecessary state updates and storage writes per unit nav. Drop the
switching branch; CASE 1 now just marks the ref and returns.
- CASE 3 (RIGHT panels available + currentSidebar=null) was auto-opening the
first RIGHT panel on a wide viewport, conflicting with expected behavior #3
("nothing open → advance → still nothing"). The closed-by-user gate above
catches the realistic null path, so this branch was state-drift recovery.
Recover to COURSE_OUTLINE (the wide-viewport default per the prior commits)
instead of opening a RIGHT panel.
Refs: openedx/wg-build-test-release#574
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: align sidebar tests with the new behavior
Tracks the source changes across the previous commits on this branch:
- storage.test.js: drop the removed outline-collapsed describes; keep the
parallel isSidebarClosedByUser / setSidebarClosedByUser coverage.
- useInitialSidebar.test.js: drop isOutlineSidebarCollapsed mock, restructure
with a "when isInitiallySidebarOpen is true and the sidebar has not been
closed by the user" wrapper around the storage-state describes. Add the
anti-flicker case (stored RIGHT panel before widget data has loaded).
- useUnitShiftBehavior.test.js: drop the isOutlineSidebarCollapsed mock, swap
in isSidebarClosedByUser, add the closed-by-user no-op, restructure the
CASE-block describes under a single precondition wrapper, and update CASE 1
and CASE 3 assertions to match the new "keep current sidebar open" /
"recover to COURSE_OUTLINE" behavior.
- useResponsiveBehavior.test.js, useSidebarSync.test.js: rename in-place from
the outline-collapsed flag to closed-by-user; add closed-by-user no-op
coverage.
- SidebarContextProvider.test.jsx: drop the outline-collapsed mocks, add
setSidebarClosedByUser assertions to the UC7* toggleSidebar tests; simplify
UC7a to just assert the initial sidebar is non-null on desktop.
- Course.test.jsx: replace the two pre-fix tests (which asserted the buggy
auto-open of discussions) with a `describe('sidebar behavior')` block
covering: default outline, closed-by-user persistence, stored-discussions /
stored-outline on render, and the four click-to-toggle transitions (close
outline, open outline closing discussions, open discussions closing outline,
close discussions).
- SidebarUnit.test.jsx: drop the two stale sessionStorage assertions; the
toggleSidebar call assertions above each one carry the meaningful coverage.
- test-utils.jsx: have setupDiscussionSidebar also return its testStore so
tests can reuse it (avoids a second initializeTestStore that resets the
discussion topics axios mock).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: open COURSE_OUTLINE on resize-to-wide instead of a RIGHT panel
useResponsiveBehavior auto-opened the first available RIGHT panel when the
viewport reached the wide breakpoint with no sidebar open, falling back to
COURSE_OUTLINE only when no RIGHT panels were available. That contradicts
the wide-viewport default in this PR (#1 in wg-build-test-release#574:
"nav opens by default") — and is the same kind of bug as the previous
unit-shift CASE 3 else-if.
Always open COURSE_OUTLINE in that branch. getFirstAvailablePanel is no
longer needed in this hook; drop the prop from SidebarContextProvider too.
Refs: openedx/wg-build-test-release#574
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: useResponsiveBehavior opens COURSE_OUTLINE on resize-to-wide
Track the source change: drop the getFirstAvailablePanel param from
buildParams, replace the two "opens X" tests (first-available vs
COURSE_OUTLINE-fallback) with a single "opens COURSE_OUTLINE on desktop
when no sidebar is open" assertion. The fallback-with-no-RIGHT-panels case
collapses into the same code path under the simplified source.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0 commit comments