feat(HelpPanelLink): Create help panel link#226
feat(HelpPanelLink): Create help panel link#226InsaneZein wants to merge 9 commits intoRedHatInsights:masterfrom
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a HelpPanelLink component and docs; exposes an imperative ref API on HelpPanelCustomTabs (openTabWithContent); updates HelpPanelContent to accept a newTab prop and call the ref; extends tab container to support customContent and url; exposes HelpPanelLink via Module Federation. Changes
Sequence DiagramsequenceDiagram
participant User
participant HelpPanelLink
participant ChromeAPI
participant HostAppHelpPanel
participant HelpPanelContent
participant HelpPanelCustomTabs
User->>HelpPanelLink: click
HelpPanelLink->>ChromeAPI: drawerActions.toggleDrawerContent(module './HelpPanel', newTab)
ChromeAPI->>HostAppHelpPanel: mount/open HelpPanel with newTab prop
HostAppHelpPanel->>HelpPanelContent: render(newTab)
HelpPanelContent->>HelpPanelCustomTabs: ref.openTabWithContent(newTab)
HelpPanelCustomTabs->>HelpPanelCustomTabs: create/select tab (customContent or url)
HelpPanelCustomTabs->>User: display tab content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/HelpPanel/HelpPanelCustomTabs.tsx`:
- Around line 234-249: Remove the debug console.log from the debounced callback
in setNewActionTitleDebounced: inside the useCallback/debounce function (symbol:
setNewActionTitleDebounced) delete the console.log({ activeTab }) call so the
debounced handler only checks newActionTitle/activeTab and calls
setNewActionTitle and updateTab with the new title; keep the debounce delay and
dependency on activeTab unchanged.
- Around line 381-386: The HelpPanelTabContainer currently isn't receiving the
customContent and url stored by openTabWithContent, so custom tabs created via
HelpPanelLink won't render their content; update the SubTabs usage to pass
customContent and url into HelpPanelTabContainer (alongside activeTabType and
setNewActionTitleDebounced), then update HelpPanelTabContainer to accept these
props and forward them to whatever component it renders for the active tab
(ensure the prop names are preserved: customContent and url) so the active Panel
receives and renders them.
🧹 Nitpick comments (1)
src/components/HelpPanel/HelpPanelContent.tsx (1)
46-51: Consider preventing duplicate tab creation on re-renders.If the parent component re-renders and passes a structurally identical but referentially different
newTabobject, this effect will create duplicate tabs. Consider tracking whether the currentnewTabhas already been opened.💡 Optional: Track opened tabs to prevent duplicates
+ const openedTabRef = useRef<string | null>(null); + // Open a new tab if newTab prop is provided useEffect(() => { - if (newTab && tabsRef.current) { + const tabKey = newTab ? `${newTab.title}-${newTab.tabType}-${newTab.url}` : null; + if (newTab && tabsRef.current && tabKey !== openedTabRef.current) { tabsRef.current.openTabWithContent(newTab); + openedTabRef.current = tabKey; } }, [newTab]);
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Can you change how the component is exposed with module federation?
|
|
||
| const setNewActionTitleDebounced: (title: string) => void = useCallback( | ||
| debounce((title: string) => { | ||
| console.log({ activeTab }); |
There was a problem hiding this comment.
No need to have console.log here
| console.log({ activeTab }); |
README.md
Outdated
| ```tsx | ||
| import { HelpPanelLink, TabType } from '@redhat-cloud-services/learning-resources'; | ||
|
|
||
| <HelpPanelLink |
There was a problem hiding this comment.
We should be using Module federation for this instead of relying on NPM. We don't have release for the application. We would have to configure it and would have to re-release with new version.
There was a problem hiding this comment.
fixed. I exposed HelpPanelLink through federated modules
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Looking good! Can you also please add unit tests and Playwright tests for this feature? Thank you.
README.md
Outdated
| import React, { lazy, Suspense } from 'react'; | ||
|
|
||
| // Lazy load the component from the federated module | ||
| const HelpPanelLink = lazy(() => |
There was a problem hiding this comment.
We are using AsyncComponent to load these components trough scalprum.
There was a problem hiding this comment.
just updated the README
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/components/HelpPanel/HelpPanelCustomTabs.tsx`:
- Around line 363-386: The SubTabs click handler updates tab.tabType/title but
leaves tab.customContent/url intact, so HelpPanelTabContainer returns early and
visible content doesn't change; modify the setActiveSubTabKey callback: when a
user selects a standard sub-tab (i.e., switching tabType) and the current tab
has pinned customContent or url, clear those fields (set customContent and url
to undefined/null) before calling updateTab and setActiveTab, or alternatively
conditionally hide the SubTabs when tab.customContent or tab.url is present;
update the logic around updateTab/setActiveTab in the SubTabs handler
(referencing SubTabs, HelpPanelTabContainer, updateTab, setActiveTab,
getSubTabTitle, setNewActionTitleDebounced) so the UI actually reflects the
chosen sub-tab.
- Around line 267-285: openTabWithContent may add a tab with an ID that already
exists, causing duplicate eventKey/key and broken Tabs selection; change
openTabWithContent to first compute newTabId (content.id ||
crypto.randomUUID()), then check the current tabs for an existing tab with that
id (use the same tabs state or a getter used by addTab), and if found update
that tab's properties (title, tabType, customContent/content, url, isNewTab) and
call setActiveTab(existingTab) instead of addTab, otherwise call addTab with the
new tab and then setActiveTab; ensure you never add a tab with an id that
already exists so React reconciliation and eventKey/key remain unique (refer to
openTabWithContent, addTab, setActiveTab, and TabDefinition).
- Around line 234-248: The debounced callback setNewActionTitleDebounced closes
over a stale newActionTitle because only activeTab is in its dependency list;
fix by switching to a ref-based access pattern: create a ref (e.g.,
newActionTitleRef) that you update whenever newActionTitle changes (via
useEffect or inside the setter), then have the debounced function read
newActionTitleRef.current instead of newActionTitle; keep activeTab in the
dependency array so the debounced function remains stable and continue to call
setNewActionTitle and updateTab with the title when conditions using activeTab
and NEW_TAB_PLACEHOLDER are met.
In `@src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsx`:
- Around line 19-29: The conditional in HelpPanelTabContainer currently uses a
truthiness check for customContent which will skip valid ReactNode values like 0
or ''—replace the check with a nullish check (e.g., if (customContent != null)
or if (customContent !== null && customContent !== undefined)) so any provided
ReactNode (including 0 and empty string) is rendered in the existing return
block that renders the div with
data-ouia-component-id="help-panel-content-container".
- Around line 31-48: Update the iframe rendered in HelpPanelTabContainer when
the url prop is present: add a sandbox attribute (e.g., empty string for maximum
restrictions or a minimal allow-list only for trusted hosts) and add
referrerPolicy="no-referrer" to prevent referrer leakage; locate the iframe
element inside HelpPanelTabContainer and modify its attributes accordingly (or
conditionally apply a less-restrictive sandbox only after
validating/allowlisting the url).
| const setNewActionTitleDebounced: (title: string) => void = useCallback( | ||
| debounce((title: string) => { | ||
| if ( | ||
| (!newActionTitle || activeTab.title === NEW_TAB_PLACEHOLDER) && | ||
| activeTab.closeable | ||
| ) { | ||
| setNewActionTitle(title); | ||
| updateTab({ | ||
| ...activeTab, | ||
| title, | ||
| }); | ||
| } | ||
| }, 2000), | ||
| [activeTab] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | head -300Repository: RedHatInsights/learning-resources
Length of output: 10027
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '224,320p'Repository: RedHatInsights/learning-resources
Length of output: 3595
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '134,151p'Repository: RedHatInsights/learning-resources
Length of output: 612
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '91,132p'Repository: RedHatInsights/learning-resources
Length of output: 1455
🏁 Script executed:
grep -n "setNewActionTitleDebounced" src/components/HelpPanel/HelpPanelCustomTabs.tsxRepository: RedHatInsights/learning-resources
Length of output: 224
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '375,390p'Repository: RedHatInsights/learning-resources
Length of output: 655
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '350,390p'Repository: RedHatInsights/learning-resources
Length of output: 1803
Debounced callback captures stale newActionTitle from closure.
The callback reads newActionTitle in the condition check (line 237) but only includes activeTab in the dependency list. This creates a stale closure: if newActionTitle changes without activeTab changing, the callback won't be recreated and will use the old captured value.
However, adding newActionTitle to the dependency array would recreate the callback on every keystroke, causing the debounce timer to reset constantly and defeating the debounce. A better approach is to read newActionTitle outside the debounced function or use a ref-based pattern to access the current value without recreating the callback.
Note: updateTab is stable and doesn't need to be added.
🤖 Prompt for AI Agents
In `@src/components/HelpPanel/HelpPanelCustomTabs.tsx` around lines 234 - 248, The
debounced callback setNewActionTitleDebounced closes over a stale newActionTitle
because only activeTab is in its dependency list; fix by switching to a
ref-based access pattern: create a ref (e.g., newActionTitleRef) that you update
whenever newActionTitle changes (via useEffect or inside the setter), then have
the debounced function read newActionTitleRef.current instead of newActionTitle;
keep activeTab in the dependency array so the debounced function remains stable
and continue to call setNewActionTitle and updateTab with the title when
conditions using activeTab and NEW_TAB_PLACEHOLDER are met.
| <SubTabs | ||
| activeSubTabKey={tab.tabType ?? TabType.learn} | ||
| setActiveSubTabKey={(tabType) => { | ||
| let newTitle = tab.title; | ||
| if (!tab.closeable) { | ||
| newTitle = getSubTabTitle(tabType); | ||
| } else if (tab.isNewTab) { | ||
| newTitle = getSubTabTitle(tabType); | ||
| } | ||
| const nextTab = { | ||
| ...tab, | ||
| tabType: tabType, | ||
| title: newTitle, | ||
| }; | ||
| updateTab(nextTab); | ||
| setActiveTab(nextTab); | ||
| }} | ||
| > | ||
| <HelpPanelTabContainer | ||
| activeTabType={tab.tabType} | ||
| setNewActionTitle={setNewActionTitleDebounced} | ||
| customContent={tab.customContent} | ||
| url={tab.url} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the HelpPanelCustomTabs.tsx file
find . -name "HelpPanelCustomTabs.tsx" -type fRepository: RedHatInsights/learning-resources
Length of output: 127
🏁 Script executed:
# Also search for HelpPanelTabContainer to understand its implementation
find . -name "*HelpPanelTabContainer*" -type fRepository: RedHatInsights/learning-resources
Length of output: 143
🏁 Script executed:
# Read the HelpPanelCustomTabs.tsx file to see the context
wc -l ./src/components/HelpPanel/HelpPanelCustomTabs.tsxRepository: RedHatInsights/learning-resources
Length of output: 131
🏁 Script executed:
# Read HelpPanelTabContainer to understand its rendering logic
wc -l ./src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsxRepository: RedHatInsights/learning-resources
Length of output: 146
🏁 Script executed:
# Read HelpPanelCustomTabs.tsx, focusing on the context around lines 363-386
cat -n ./src/components/HelpPanel/HelpPanelCustomTabs.tsx | sed -n '350,390p'Repository: RedHatInsights/learning-resources
Length of output: 1803
🏁 Script executed:
# Read HelpPanelTabContainer.tsx entirely since it's small
cat -n ./src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsxRepository: RedHatInsights/learning-resources
Length of output: 2035
Sub-tabs remain interactive even when custom content is pinned.
HelpPanelTabContainer prioritizes customContent/url via early returns, so clicking sub-tabs updates the title/type but the visible content doesn't change. Consider hiding sub-tabs for those tabs, or clearing custom content when a standard sub-tab is chosen.
💡 One possible approach
- <SubTabs
- activeSubTabKey={tab.tabType ?? TabType.learn}
- setActiveSubTabKey={(tabType) => {
- let newTitle = tab.title;
- if (!tab.closeable) {
- newTitle = getSubTabTitle(tabType);
- } else if (tab.isNewTab) {
- newTitle = getSubTabTitle(tabType);
- }
- const nextTab = {
- ...tab,
- tabType: tabType,
- title: newTitle,
- };
- updateTab(nextTab);
- setActiveTab(nextTab);
- }}
- >
- <HelpPanelTabContainer
- activeTabType={tab.tabType}
- setNewActionTitle={setNewActionTitleDebounced}
- customContent={tab.customContent}
- url={tab.url}
- />
- </SubTabs>
+ {tab.customContent || tab.url ? (
+ <HelpPanelTabContainer
+ activeTabType={tab.tabType}
+ setNewActionTitle={setNewActionTitleDebounced}
+ customContent={tab.customContent}
+ url={tab.url}
+ />
+ ) : (
+ <SubTabs
+ activeSubTabKey={tab.tabType ?? TabType.learn}
+ setActiveSubTabKey={(tabType) => {
+ let newTitle = tab.title;
+ if (!tab.closeable) {
+ newTitle = getSubTabTitle(tabType);
+ } else if (tab.isNewTab) {
+ newTitle = getSubTabTitle(tabType);
+ }
+ const nextTab = {
+ ...tab,
+ tabType: tabType,
+ title: newTitle,
+ };
+ updateTab(nextTab);
+ setActiveTab(nextTab);
+ }}
+ >
+ <HelpPanelTabContainer
+ activeTabType={tab.tabType}
+ setNewActionTitle={setNewActionTitleDebounced}
+ customContent={tab.customContent}
+ url={tab.url}
+ />
+ </SubTabs>
+ )}🤖 Prompt for AI Agents
In `@src/components/HelpPanel/HelpPanelCustomTabs.tsx` around lines 363 - 386, The
SubTabs click handler updates tab.tabType/title but leaves tab.customContent/url
intact, so HelpPanelTabContainer returns early and visible content doesn't
change; modify the setActiveSubTabKey callback: when a user selects a standard
sub-tab (i.e., switching tabType) and the current tab has pinned customContent
or url, clear those fields (set customContent and url to undefined/null) before
calling updateTab and setActiveTab, or alternatively conditionally hide the
SubTabs when tab.customContent or tab.url is present; update the logic around
updateTab/setActiveTab in the SubTabs handler (referencing SubTabs,
HelpPanelTabContainer, updateTab, setActiveTab, getSubTabTitle,
setNewActionTitleDebounced) so the UI actually reflects the chosen sub-tab.
| // If custom content is provided, render it directly | ||
| if (customContent) { | ||
| return ( | ||
| <div | ||
| className="pf-v6-u-p-md" | ||
| data-ouia-component-id="help-panel-content-container" | ||
| > | ||
| {customContent} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsxRepository: RedHatInsights/learning-resources
Length of output: 2035
Use a nullish check instead of truthiness check for customContent.
The current if (customContent) skips valid ReactNode values like 0 or empty strings, which would incorrectly fall through to the URL/default renderer. Since customContent is typed as ReactNode, these values can be legitimately provided and should be rendered.
🔧 Suggested fix
- if (customContent) {
+ if (customContent !== undefined && customContent !== null) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If custom content is provided, render it directly | |
| if (customContent) { | |
| return ( | |
| <div | |
| className="pf-v6-u-p-md" | |
| data-ouia-component-id="help-panel-content-container" | |
| > | |
| {customContent} | |
| </div> | |
| ); | |
| } | |
| // If custom content is provided, render it directly | |
| if (customContent !== undefined && customContent !== null) { | |
| return ( | |
| <div | |
| className="pf-v6-u-p-md" | |
| data-ouia-component-id="help-panel-content-container" | |
| > | |
| {customContent} | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsx` around
lines 19 - 29, The conditional in HelpPanelTabContainer currently uses a
truthiness check for customContent which will skip valid ReactNode values like 0
or ''—replace the check with a nullish check (e.g., if (customContent != null)
or if (customContent !== null && customContent !== undefined)) so any provided
ReactNode (including 0 and empty string) is rendered in the existing return
block that renders the div with
data-ouia-component-id="help-panel-content-container".
| // If URL is provided, render it in an iframe | ||
| if (url) { | ||
| return ( | ||
| <div | ||
| className="pf-v6-u-p-md" | ||
| data-ouia-component-id="help-panel-content-container" | ||
| style={{ height: '100%', display: 'flex', flexDirection: 'column' }} | ||
| > | ||
| <iframe | ||
| src={url} | ||
| style={{ | ||
| width: '100%', | ||
| height: '100%', | ||
| border: 'none', | ||
| flex: 1, | ||
| }} | ||
| title="External content" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "HelpPanelTabContainer.tsx" -type fRepository: RedHatInsights/learning-resources
Length of output: 143
🏁 Script executed:
cat -n ./src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsxRepository: RedHatInsights/learning-resources
Length of output: 2035
🏁 Script executed:
rg "HelpPanelTabContainer" --type tsx --type ts -B 2 -A 2Repository: RedHatInsights/learning-resources
Length of output: 104
🏁 Script executed:
rg "HelpPanelTabContainer" -B 2 -A 2Repository: RedHatInsights/learning-resources
Length of output: 1695
🏁 Script executed:
cat -n ./src/components/HelpPanel/HelpPanelCustomTabs.tsxRepository: RedHatInsights/learning-resources
Length of output: 13726
🏁 Script executed:
cat -n ./src/components/HelpPanel/HelpPanelLink.tsxRepository: RedHatInsights/learning-resources
Length of output: 3814
🏁 Script executed:
rg "HelpPanelLink" -A 5 -B 2 | head -100Repository: RedHatInsights/learning-resources
Length of output: 5943
🏁 Script executed:
rg "openTabWithContent\|url" ./src/components/HelpPanel/ -B 2 -A 2Repository: RedHatInsights/learning-resources
Length of output: 59
🏁 Script executed:
find ./src/components/HelpPanel -type f -name "*.tsx" -o -name "*.ts"Repository: RedHatInsights/learning-resources
Length of output: 665
🏁 Script executed:
cat -n ./src/components/HelpPanel/HelpPanelContent.tsxRepository: RedHatInsights/learning-resources
Length of output: 5472
Add sandbox and referrerPolicy attributes to iframe for untrusted URLs.
The url prop originates from external developers via the Module Federation-exported HelpPanelLink component with no validation or allowlisting. An iframe rendering untrusted URLs without sandbox restrictions can execute active content or navigate the top-level page. Add sandbox and referrerPolicy attributes to restrict capabilities.
Suggested fix
<iframe
src={url}
+ sandbox="allow-same-origin allow-scripts"
+ referrerPolicy="no-referrer"
style={{
width: '100%',
height: '100%',
border: 'none',
flex: 1,
}}
title="External content"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If URL is provided, render it in an iframe | |
| if (url) { | |
| return ( | |
| <div | |
| className="pf-v6-u-p-md" | |
| data-ouia-component-id="help-panel-content-container" | |
| style={{ height: '100%', display: 'flex', flexDirection: 'column' }} | |
| > | |
| <iframe | |
| src={url} | |
| style={{ | |
| width: '100%', | |
| height: '100%', | |
| border: 'none', | |
| flex: 1, | |
| }} | |
| title="External content" | |
| /> | |
| // If URL is provided, render it in an iframe | |
| if (url) { | |
| return ( | |
| <div | |
| className="pf-v6-u-p-md" | |
| data-ouia-component-id="help-panel-content-container" | |
| style={{ height: '100%', display: 'flex', flexDirection: 'column' }} | |
| > | |
| <iframe | |
| src={url} | |
| sandbox="allow-same-origin allow-scripts" | |
| referrerPolicy="no-referrer" | |
| style={{ | |
| width: '100%', | |
| height: '100%', | |
| border: 'none', | |
| flex: 1, | |
| }} | |
| title="External content" | |
| /> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/components/HelpPanel/HelpPanelTabs/HelpPanelTabContainer.tsx` around
lines 31 - 48, Update the iframe rendered in HelpPanelTabContainer when the url
prop is present: add a sandbox attribute (e.g., empty string for maximum
restrictions or a minimal allow-list only for trusted hosts) and add
referrerPolicy="no-referrer" to prevent referrer leakage; locate the iframe
element inside HelpPanelTabContainer and modify its attributes accordingly (or
conditionally apply a less-restrictive sandbox only after
validating/allowlisting the url).
|
@karelhala I've run the playwright test a bunch of times locally and they pass each time. Not sure what's causing them to fail in konflux. |
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Looking good, just a question wrt iframe. Might need rebasing as well.
| data-ouia-component-id="help-panel-content-container" | ||
| style={{ height: '100%', display: 'flex', flexDirection: 'column' }} | ||
| > | ||
| <iframe |
There was a problem hiding this comment.
What is the usecase for iframe? Is there a requirement for it right now?
There was a problem hiding this comment.
I thought we needed this to view the content that's passed in through the URL prop. The doc mentions opening pages tied to specific pages in the UI. Maybe I've misunderstood.
There was a problem hiding this comment.
That's out of scope for now. The context panel is not on the roadmpa at the moment.
2 participants
Adds the HelpPanelLink component that allows you to create a link that will open up the Help Panel drawer with a specific tab.
Screencast.From.2026-01-29.11-01-35.mp4