Rework quickstarts implementation in the help panel#320
Conversation
Summary by CodeRabbit
WalkthroughThis PR migrates quickstart handling in the Help Panel from an overlay-based architecture to an in-panel drill-down navigation model. The container stops fetching and managing quickstart overlays; instead, it routes pending opens to the Learn tab. LearnPanel now manages the drill-down state, bookmarks, and conditional rendering between the resource list and active quickstart detail views. ChangesQuickstart Drill-Down Migration
Sequence DiagramsequenceDiagram
participant User
participant HelpPanelCustomTabs
participant LearnPanel
participant openQuickstartStore
participant QuickStartController
participant API
User->>HelpPanelCustomTabs: Quickstart open event
HelpPanelCustomTabs->>LearnPanel: Switch to Learn tab
HelpPanelCustomTabs->>openQuickstartStore: Leave event unconsumed
openQuickstartStore->>LearnPanel: pendingOpen detected
LearnPanel->>LearnPanel: Initialize activeQuickStartID & state
LearnPanel->>openQuickstartStore: Consume event
LearnPanel->>LearnPanel: Render drill-down UI
User->>LearnPanel: Click bookmark toggle
LearnPanel->>LearnPanel: Optimistic local update
LearnPanel->>API: POST to favorites
alt Success
LearnPanel->>LearnPanel: Keep bookmark state
else Error
LearnPanel->>LearnPanel: Revert to previous state
end
User->>LearnPanel: Click breadcrumb back
LearnPanel->>LearnPanel: Return to list mode
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx (2)
484-493: 💤 Low valuePrefer functional update for state consistency.
handleQuickStartClickuses direct state spread while the similar logic in theuseEffect(lines 230-233) uses functional update. Using the callback form ensures the latest state is read, avoiding potential stale reads on rapid clicks.♻️ Suggested fix
const handleQuickStartClick = (quickstartId: string) => { setActiveQuickStartID(quickstartId); // Initialize state if needed if (!allQuickStartStates[quickstartId]) { - setAllQuickStartStates({ - ...allQuickStartStates, + setAllQuickStartStates((prev) => ({ + ...prev, [quickstartId]: getDefaultQuickStartState(), - }); + })); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx` around lines 484 - 493, handleQuickStartClick currently spreads the possibly stale allQuickStartStates when calling setAllQuickStartStates; change it to use the functional updater to avoid race conditions (e.g., call setAllQuickStartStates(prev => ({ ...prev, [quickstartId]: getDefaultQuickStartState() })) only when the key is missing). Keep setActiveQuickStartID as-is or also switch to its functional form if desired; reference setAllQuickStartStates, allQuickStartStates, handleQuickStartClick, and getDefaultQuickStartState to locate the change.
171-173: 💤 Low valueUnused
setNewActionTitleparameter.The
setNewActionTitleprop is declared but never used inLearnPanelContent. If it's kept only for API compatibility withHelpPanelTabContainer, consider adding a comment or using_setNewActionTitlenaming convention to indicate intentional omission.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx` around lines 171 - 173, The LearnPanelContent component declares a prop setNewActionTitle that is never used; either remove it from LearnPanelContent's props if it's not needed, or explicitly mark it as intentionally unused by renaming it to _setNewActionTitle (or adding a short comment) to indicate API compatibility with HelpPanelTabContainer. Update the function signature for LearnPanelContent and any callers to match the chosen approach (remove the prop everywhere or use the underscore-prefixed name) so linter warnings are resolved and intent is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/HelpPanel/HelpPanelTabs/LearnPanel.stories.tsx`:
- Around line 1120-1134: The current waitFor assertion uses a logical OR which
doesn't verify state preservation; update the assertion inside the waitFor
callback to explicitly assert that continueButton (the query result assigned to
`continueButton`) is present and that startButtonAgain (the query result
assigned to `startButtonAgain`) is not present, i.e., replace the single
`expect(continueButton || startButtonAgain).toBeInTheDocument()` with two
assertions that `continueButton` is in the document and `startButtonAgain` is
not, keeping the same waitFor, canvas queries, and timeout.
- Line 227: The story currently sets an icon property to a JSX element (icon:
<span aria-hidden />) which cannot be JSON-serialized; replace that JSX with a
serializable representation (e.g., a string key or URL) used by your
API/tests—update the object in LearnPanel.stories.tsx where the icon field is
defined to use a string identifier (like "help" or an icon URL) instead of the
JSX element so HttpResponse.json() returns the expected value.
---
Nitpick comments:
In `@src/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx`:
- Around line 484-493: handleQuickStartClick currently spreads the possibly
stale allQuickStartStates when calling setAllQuickStartStates; change it to use
the functional updater to avoid race conditions (e.g., call
setAllQuickStartStates(prev => ({ ...prev, [quickstartId]:
getDefaultQuickStartState() })) only when the key is missing). Keep
setActiveQuickStartID as-is or also switch to its functional form if desired;
reference setAllQuickStartStates, allQuickStartStates, handleQuickStartClick,
and getDefaultQuickStartState to locate the change.
- Around line 171-173: The LearnPanelContent component declares a prop
setNewActionTitle that is never used; either remove it from LearnPanelContent's
props if it's not needed, or explicitly mark it as intentionally unused by
renaming it to _setNewActionTitle (or adding a short comment) to indicate API
compatibility with HelpPanelTabContainer. Update the function signature for
LearnPanelContent and any callers to match the chosen approach (remove the prop
everywhere or use the underscore-prefixed name) so linter warnings are resolved
and intent is clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2713af8b-1bb7-4613-a36c-036d7c4618da
📒 Files selected for processing (5)
src/components/HelpPanel/HelpPanelContent.tsxsrc/components/HelpPanel/HelpPanelCustomTabs.tsxsrc/components/HelpPanel/HelpPanelTabs/LearnPanel.stories.tsxsrc/components/HelpPanel/HelpPanelTabs/LearnPanel.tsxsrc/components/HelpPanel/HelpPanelTabs/SearchPanel/SearchResultItem.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx`:
- Around line 616-633: Replace hardcoded UI/ARIA strings with entries from the
i18n messages catalog: extract "Add bookmark" / "Remove bookmark" used in the
aria-label for the bookmark control, the fallback "Quick start" used in
activeQuickStart.spec.type?.text, the unit "minutes" shown next to
activeQuickStart.spec.durationMinutes, and other hardcoded labels like "Filter
by scope" and "Learning resources" into message keys in the messages file; then
update the JSX in LearnPanel (references: activeQuickStart, aria-label on the
bookmark control, Title displayName, and the duration span) to use the
appropriate message lookups (e.g., messages.quickStartFallback,
messages.addBookmark, messages.removeBookmark, messages.minutes, etc.) so the UI
uses localized strings instead of literals.
- Around line 511-557: The bookmark toggle handler
(handleActiveQuickStartBookmark) is vulnerable to race conditions from rapid
clicks; guard against repeated clicks by tracking in-flight updates (e.g., an
isTogglingBookmarkRef or a map keyed by quickstart name) and early-return when a
request is pending, compute the optimistic new favorite from the previous state
inside setAllQuickStarts's updater (not from activeQuickStart render value), set
the in-flight flag before sending the axios POST and clear it in both success
and catch branches, and call handleBookmarkItemToggle after success (or still
schedule a background refresh) so concurrent clicks are ignored; apply the same
in-flight guarding and updater-change to the other toggle handler referenced
around lines 605-621 (the per-item bookmark toggle).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: f44d374b-6ff6-4e47-b4b6-d5b0b5714a8f
📒 Files selected for processing (2)
src/components/HelpPanel/HelpPanelTabs/LearnPanel.stories.tsxsrc/components/HelpPanel/HelpPanelTabs/LearnPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/HelpPanel/HelpPanelTabs/LearnPanel.stories.tsx
For RHCLOUD-47506. Changes opening quickstarts in the help panel from an overlay to a part of the Learn panel. Quickstarts will now open (from the Search Panel or Learn) as a "drill-down" style article where the breadcrumb navigates back to the Learn Panel.
Screen.Recording.2026-05-14.at.4.06.20.PM.mov