feat(helppanel): init spur fixes#236
Open
aferd wants to merge 6 commits intoRedHatInsights:masterfrom
Open
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 i18n to help-panel labels and sub-tab titles, introduces an AiExperienceIcon component and replaces AskRedHatIcon usage, computes user-friendly bundle display names with deduplication for API docs, applies a compact prop to several ToggleGroups, expands help-panel white-background styling, updates FiltersMetadata, and modifies Jest test setup and config + tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@src/components/HelpPanel/HelpPanelContent.tsx`:
- Line 76: In the HelpPanelContent component update the user-facing button
labels that read "Chat with an assitant" to the correct spelling "Chat with an
assistant"; locate the JSX strings in HelpPanelContent.tsx (both occurrences
referenced in the review at lines 76 and 89) and replace the misspelled text so
both buttons display "assistant" consistently.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/common/AiExperienceIcon.tsx`:
- Around line 9-19: The AiExperienceIcon component renders a decorative SVG that
should be ignored by assistive tech; update the <svg> element in
AiExperienceIcon (which uses props className, width, height and iconPath) to
include aria-hidden="true" so screen readers won’t announce it (optionally also
add focusable="false" for legacy browsers).
In `@src/components/HelpPanel/HelpPanelCustomTabs.scss`:
- Around line 8-35: The PF6-only override leaves PF5 drawer headers grey and the
negative margins on .pf-v6-c-data-list can cause horizontal scrolling; add the
PF5 selector .pf-c-drawer__head alongside .pf-v6-c-drawer__head (and/or target
.lr-c-help-panel-custom-tabs .pf-c-drawer__head) to set the white background,
and prevent horizontal overflow by changing the negative margin/width strategy
on [data-ouia-component-id="help-panel-content-container"] .pf-v6-c-data-list
(either clamp the margin-inline with max(..., 0) or replace the calc-based width
with a safe max-width/min-width approach, or add overflow-x: hidden to
[data-ouia-component-id="help-panel-content-container"]) so the layout doesn’t
produce a horizontal scrollbar.
🧹 Nitpick comments (1)
src/components/HelpPanel/HelpPanelTabs/APIPanel.tsx (1)
32-43: Consider reusing the shared bundle display‑name helper to avoid drift.There’s already a
getBundleDisplayNameinsrc/utils/bundleUtils.ts. Wrapping it here keeps the chrome title fallback while centralizing the mapping logic.♻️ Suggested refactor
-import { FiltersMetadata } from '../../../utils/FiltersCategoryInterface'; +import { getBundleDisplayName as getBundleDisplayNameFromMetadata } from '../../../utils/bundleUtils'; -const getBundleDisplayName = ( - bundleId: string, - chromeTitle: string | undefined -): string => { - const mapped = FiltersMetadata[bundleId]; - if (mapped) return mapped.split(' (')[0]; - if (chromeTitle) return chromeTitle; - return bundleId.charAt(0).toUpperCase() + bundleId.slice(1); -}; +const getBundleDisplayName = ( + bundleId: string, + chromeTitle: string | undefined +): string => + getBundleDisplayNameFromMetadata(bundleId, { allowFallback: false }) ?? + chromeTitle ?? + getBundleDisplayNameFromMetadata(bundleId);Also applies to: 181-184
apinkert
reviewed
Feb 13, 2026
karelhala
reviewed
Feb 16, 2026
Contributor
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Can you please add some unit tests and playwright tests? You can also add some component tests if you'd want.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/HelpPanel/HelpPanelCustomTabs.test.tsx (1)
11-17: Consider makinguseFlagmock more explicit.The current mock always returns
trueregardless of which flag is queried. While this works for these structural tests, it could mask issues if component behavior varies by flag.♻️ Proposed improvement for more explicit flag mocking
jest.mock('@unleash/proxy-client-react', () => ({ - useFlag: () => true, + useFlag: (flag: string) => { + const enabledFlags = [ + 'platform.chrome.help-panel_search', + 'platform.chrome.help-panel_knowledge-base', + ]; + return enabledFlags.includes(flag); + }, useFlags: () => [ { name: 'platform.chrome.help-panel_search', enabled: true }, { name: 'platform.chrome.help-panel_knowledge-base', enabled: true }, ], }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelCustomTabs.test.tsx` around lines 11 - 17, The mock for useFlag is too generic—change the jest.mock implementation so useFlag accepts a flag name and returns a value based on an explicit map instead of always true; e.g., implement useFlag: (name) => flagMap[name] ?? false and define flagMap entries for 'platform.chrome.help-panel_search' and 'platform.chrome.help-panel_knowledge-base' (keep useFlags as-is or derive it from the same map) so tests reflect per-flag behavior and avoid masking flag-specific bugs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/HelpPanel/HelpPanelCustomTabs.test.tsx`:
- Around line 11-17: The mock for useFlag is too generic—change the jest.mock
implementation so useFlag accepts a flag name and returns a value based on an
explicit map instead of always true; e.g., implement useFlag: (name) =>
flagMap[name] ?? false and define flagMap entries for
'platform.chrome.help-panel_search' and
'platform.chrome.help-panel_knowledge-base' (keep useFlags as-is or derive it
from the same map) so tests reflect per-flag behavior and avoid masking
flag-specific bugs.
Collaborator
Author
|
/retest |
karelhala
reviewed
Feb 23, 2026
Contributor
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Codewise looks really good! Thank you for adding the unit tests for HelpCustomTabs.
Contributor
There was a problem hiding this comment.
Thank you for adding these tests! Can you also add some tests on interacting with the UI?
Comment on lines
+35
to
+43
| const getBundleDisplayName = ( | ||
| bundleId: string, | ||
| chromeTitle: string | undefined | ||
| ): string => { | ||
| const mapped = FiltersMetadata[bundleId]; | ||
| if (mapped) return mapped.split(' (')[0]; | ||
| if (chromeTitle) return chromeTitle; | ||
| return bundleId.charAt(0).toUpperCase() + bundleId.slice(1); | ||
| }; |
Contributor
There was a problem hiding this comment.
This function would be better outside of the component so it's not constantly recreated on each re-render.
karelhala
reviewed
Feb 23, 2026
Contributor
There was a problem hiding this comment.
Can we also add unit tests for the new items?
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.
https://issues.redhat.com/browse/RHCLOUD-45014
✅ 1. Update text to Chat with an assistant and update icon
✅ 2. fix buggy background
3. Apps need to update their frontend.yml configs with API endpoints in order for us to show them in the APIs tab. Not sure how we want to proceed with getting that done.
✅ 4. use compact version for toggle group
✅ 5. fix width
✅ 6. same buggy background
✅ 7. update bundle toggle to say RHEL and not insights
Screen.Recording.2026-02-13.at.1.34.23.PM.mov