fix: don't auto-open right sidebar by default (backport #1923)#1930
Merged
brian-smith-tcril merged 1 commit intoJun 17, 2026
Merged
Conversation
brian-smith-tcril
commented
Jun 16, 2026
brian-smith-tcril
commented
Contributor
- fix: don't auto-open right sidebar by default #1923
- [Test failure] TC-00047: Right sidebar always opened when accessing in-course experience page wg-build-test-release#574
* fix: don't auto-open right sidebar by default The navigation (course outline) sidebar should be the default; PR openedx#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 openedx#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 openedx#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 (openedx#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>
brian-smith-tcril
added
the
create-sandbox
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/verawood #1930 +/- ##
=================================================
Coverage 91.29% 91.30%
=================================================
Files 343 346 +3
Lines 5770 5776 +6
Branches 1388 1350 -38
=================================================
+ Hits 5268 5274 +6
Misses 483 483
Partials 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@brian-smith-tcril - this looks great from the learner perspective. Thank you for fixing this. |
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
3 participants
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.