Conversation
ciiay
requested review from
a team,
karthikjeeyar,
rohitkrai03 and
yangcao77
as code owners
September 14, 2026 22:16
Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4753 +/- ##
=======================================
Coverage 63.26% 63.27%
=======================================
Files 2675 2679 +4
Lines 106439 106526 +87
Branches 29778 29781 +3
=======================================
+ Hits 67340 67399 +59
- Misses 38539 38569 +30
+ Partials 560 558 -2
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ciiay
force-pushed
the
rhdhbugs-3733-fix-IA-ui
branch
from
September 15, 2026 12:27
283a98b to
1d6de18
Compare
Member
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 11:11 AM UTC · Completed 11:16 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $1.95 |
| }, | ||
| '& > .pf-v6-c-divider, & > .pf-v5-c-divider': { | ||
| display: 'none', | ||
| }, |
Member
There was a problem hiding this comment.
If we can't avoid these overrides please raise upstream issue and put issue link as comment.
Review — commentPR: #4753 — fix(intelligent-assistant): restore IA UI styling for RHDHBUGS-3733 SummaryThis PR restores Intelligent Assistant UI styling after PatternFly theme overrides broke layout across overlay, docked, and fullscreen modes. The change introduces a shared Overall this is a well-structured refactor with consistent styling patterns. Tests (unit and e2e) are updated to match. A patch changeset is correctly included. Findings1 · i18n regression — hardcoded "Close" aria-label on MCP modal (medium)The MCP configure-server modal close button's // McpConfigureServerModal.tsx (new)
<ConfigureModalCloseButton
className={MCP_CONFIGURE_MODAL_CLOSE_CLASS}
aria-label="Close" // was t('mcp.settings.closeConfigureModalAriaLabel')
icon={<TimesIcon />}
variant="plain"
onClick={close}
/>The translation key The corresponding e2e test helper Remediation: Use the existing translation key: 2 · Commented-out code (low)In [`& .${mcpClasses.closeButton}`]: {
...compactPlainCircleButtonCss,
// minWidth: '2.5rem !important',
width: '2.25rem !important',
...This should be removed to keep the code clean. Notes
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
requires-manual-review
Member
There was a problem hiding this comment.
We can't have so much styles override, override is only accepted for must be fox styles issues. Other styles which is not matching prototype is fine we can raise upstream issues and get it done there. Adding this much styles override will be very hard to maintain.
Comment on lines
+274
to
+286
| border: contentBorder, | ||
| borderRadius: 24, | ||
| padding: theme.spacing(0.5), | ||
| '&::after': { | ||
| display: 'none', | ||
| }, | ||
| }, | ||
| ...messageBarActionsAlignCss, | ||
| [messageBarAttachMicrophoneSelector]: messageBarAttachMicrophoneButtonCss, | ||
| [messageBarMicrophoneActiveSelector]: messageBarMicrophoneActiveButtonCss, | ||
| [messageBarSendStopSelector]: { | ||
| ...messageBarSendStopButtonCss, | ||
| borderRadius: 'var(--pf-t--global--border--radius--pill) !important', |
Member
There was a problem hiding this comment.
If we can't avoid these overrides please raise upstream issue and put issue link as comment.
Comment on lines
+164
to
+222
| const chatHistoryDrawerCollapseCloseCss = { | ||
| '& .pf-v6-c-drawer__close, & .pf-v5-c-drawer__close': { | ||
| marginTop: 0, | ||
| marginRight: 0, | ||
| }, | ||
| '& .pf-v6-c-drawer__close .pf-v6-c-button svg, & .pf-v5-c-drawer__close .pf-v5-c-button svg': | ||
| { | ||
| display: 'none', | ||
| }, | ||
| '& .pf-v6-c-drawer__close .pf-v6-c-button, & .pf-v5-c-drawer__close .pf-v5-c-button': | ||
| { | ||
| ...drawerCollapseButtonSizeCss, | ||
| ...compactPlainIconButtonRadiusCss, | ||
| ...plainCircleButtonAfterCss, | ||
| position: 'relative', | ||
| display: 'inline-flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| lineHeight: 0, | ||
| '--pf-v6-c-button--BorderWidth': '0', | ||
| '--pf-v6-c-button--m-plain--BorderWidth': '0', | ||
| '--pf-v6-c-button--m-plain--hover--BorderWidth': '0', | ||
| '--pf-v6-c-button--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--default)', | ||
| '--pf-v6-c-button--hover--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--hover)', | ||
| '--pf-v6-c-button--m-plain--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--default)', | ||
| '--pf-v6-c-button--m-plain--hover--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--hover)', | ||
| '&:hover:not(:disabled), &:focus-visible:not(:disabled)': { | ||
| ...compactPlainIconButtonRadiusCss, | ||
| backgroundColor: | ||
| 'var(--pf-t--global--background--color--action--plain--hover) !important', | ||
| '--pf-v6-c-button--hover--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--hover)', | ||
| '--pf-v6-c-button--m-plain--hover--BackgroundColor': | ||
| 'var(--pf-t--global--background--color--action--plain--hover)', | ||
| }, | ||
| '& .pf-v6-c-button__icon, & .pf-v5-c-button__icon': | ||
| drawerCollapseIconSlotCss, | ||
| '& .pf-v6-c-button__icon::before, & .pf-v5-c-button__icon::before': { | ||
| content: '""', | ||
| display: 'block', | ||
| width: 24, | ||
| height: 24, | ||
| flexShrink: 0, | ||
| mask: COLLAPSE_PANEL_ICON_SVG, | ||
| WebkitMask: COLLAPSE_PANEL_ICON_SVG, | ||
| maskSize: 'contain', | ||
| WebkitMaskSize: 'contain', | ||
| maskRepeat: 'no-repeat', | ||
| WebkitMaskRepeat: 'no-repeat', | ||
| maskPosition: 'center', | ||
| WebkitMaskPosition: 'center', | ||
| backgroundColor: 'currentColor', | ||
| }, | ||
| }, | ||
| } as const; |
Member
There was a problem hiding this comment.
We can't take this much overrides, lets discuss what all styles will break without these overrides.
Comment on lines
+273
to
+286
| '&.pf-chatbot--embedded': { | ||
| overflow: 'hidden', | ||
| boxShadow: 'none !important', | ||
| ...(isDockedMode | ||
| ? { | ||
| border: 'none !important', | ||
| borderInlineStart: `${contentBorder} !important`, | ||
| borderRadius: 0, | ||
| } | ||
| : { | ||
| border: `${contentBorder} !important`, | ||
| borderRadius: isCompact | ||
| ? 'var(--pf-t--global--border--radius--medium)' | ||
| : '1rem', |
Member
There was a problem hiding this comment.
If we can't avoid these overrides please raise upstream issue and put issue link as comment.
Comment on lines
+298
to
+303
| '& .pf-chatbot__header .pf-v6-c-menu-toggle.pf-chatbot__button--toggle-options, & .pf-chatbot__header .pf-chatbot__button--toggle-options': | ||
| chatHeaderOptionsToggleCss, | ||
| '& .pf-chatbot__history': { | ||
| ...chatHistoryDrawerCollapseCloseCss, | ||
| // History drawer wraps main chat too — scope icon toggles to search/sort and row kebabs only. | ||
| '& .pf-chatbot__history-search-actions .pf-v6-c-menu-toggle, & .pf-chatbot__history-search-actions .pf-v5-c-menu-toggle, & .pf-chatbot__history-actions .pf-v6-c-menu-toggle, & .pf-chatbot__history-actions .pf-v5-c-menu-toggle, & .pf-chatbot__menu-item .pf-v6-c-menu-toggle, & .pf-chatbot__menu-item .pf-v5-c-menu-toggle': |
Member
There was a problem hiding this comment.
If we can't avoid these overrides please raise upstream issue and put issue link as comment.
Align chat, dock, MCP, notebooks, and message bar controls with the intended prototype after MUI migration regressions, using shared plain icon styling and stable compact MCP/history layout. Co-authored-by: Cursor <cursoragent@cursor.com>
Center the file type badge, filename, and kebab menu on one row in the notebook documents sidebar. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Yi Cai <yicai@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ator Update e2e locators to match the hardcoded aria-label "Close" on the configure modal close button. Assisted-by: Auto Co-authored-by: Cursor <cursoragent@cursor.com>
… tab switch Share scroll overflow detection and centered jump-button layout between chat and notebook message views so back-to-top controls stay visible and aligned when switching tabs. Fixes: https://redhat.atlassian.net/browse/RHDHBUGS-3733 Signed-off-by: Yi Cai <yicai@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Yi Cai <yicai@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Use translated aria-label in the modal and align unit/e2e locators with the message key. Co-authored-by: Cursor <cursoragent@cursor.com>
ciiay
force-pushed
the
rhdhbugs-3733-fix-IA-ui
branch
from
September 16, 2026 13:23
c218464 to
9ce40b0
Compare
StyledChatbotContent references ChatbotContent; missing import broke the IA app at runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the chat history nav close button aria-label after rebase onto saved prompts. Co-authored-by: Cursor <cursoragent@cursor.com>
…aved prompts Re-merge pre-rebase IA layout (docked borders, history drawer controls, CompactPlainIconButton header actions, message bar footer chrome) while keeping SettingsPanel and scroll jump hooks. Omit attach/mic hover suppression so message bar matches PF chatbot plain icon behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
…verrides Replace custom PlainIconButton wrappers with stock PatternFly plain buttons, keep layout-only message bar and drawer helpers, and fix chat history drawer close styling (chatbot pill sizing and duplicate icon in fullscreen). Co-authored-by: Cursor <cursoragent@cursor.com>
…kens Fix docked MCP server row edit hover, table width alignment, and PF6 pencil rendering; consolidate floating-shell and duplicate-icon CSS into chatShellTokens for easier PR comparison with main. Co-authored-by: Cursor <cursoragent@cursor.com>
…able props Use PF plain table mode, cell layout styles, row hover edit, and wrapped name/status cells while reducing grid and shell padding overrides. Co-authored-by: Cursor <cursoragent@cursor.com>
Rely on PF compact table row padding, move toggle inset to cell layout styles, and dedupe sort header button styling. Co-authored-by: Cursor <cursoragent@cursor.com>
Use PF table button markup for sort headers with SortAmount icons, and middle-align body cells when name or status text wraps. Co-authored-by: Cursor <cursoragent@cursor.com>
Add @patternfly/react-styles and load table class maps from the Table component module so McpTableSortHeader resolves at runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply pf6HideNestedRhUiIconCss on docked and fullscreen MCP settings containers so table edit buttons no longer need local overrides. Co-authored-by: Cursor <cursoragent@cursor.com>
Move backdrop stacking from global selectors in MCP settings to the configure modal backdropClassName and PF backdrop z-index tokens. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop custom active/inactive sort icon color classes and use PatternFly table sort selected state for MCP column headers. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unused PlainIconButton message-bar tokens, drop footer-container width hacks in chat and notebook views, and style the model selector locally. Co-authored-by: Cursor <cursoragent@cursor.com>
…names Playwright looked for tooltip collapse labels, but the UI exposes aria.chatHistoryMenu and aria.closeDrawerPanel. Centralize drawer helpers so display mode and sidebar tests open and close history reliably. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Member
Author
|
Closing this in favor of #5003 |
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
2 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.
Description
After the PatternFly update, global theme styles were overriding Intelligent Assistant-specific layout and control styling. This PR restores intended IA chrome across overlay, docked, and fullscreen modes while preferring PatternFly defaults where custom CSS is no longer needed (message bar controls, table sort indicators, and several footer width hacks were trimmed in follow-up commits).
Fixed
Change list
chatShellTokens.tsandPlainIconButton.tsx(drawer collapse sizing/icons, chat history drawer close affordance, shared message bar shell CSS); consolidate duplicated shell rules out ofLightSpeedChat.tsxwhere practical.MessageBarModelSelectorand remove redundant custom CSS for attach/mic/send/stop where PF layout is sufficient.aria.chatHistoryMenu) and drawer panel close (aria.closeDrawerPanel); refreshCollapsedHistoryStrip(expand / quick new chat).Tablewith sortable headers via@patternfly/react-styles(McpTableSortHeader); row/column alignment and hover edit control; plain icon buttons for panel/table actions; scroll jump buttons when content overflows.backdropClassName(remove global backdropGlobalStylesfrom MCP settings).pf6HideNestedRhUiIconCsson settings shells (MCP / saved prompts) so nested RH UI glyphs do not double-render pencil icons.useChatContentScrollOverflow,chatMessageScrollLayout).chatHistoryDrawer.tsusesaria.chatHistoryMenu/aria.closeDrawerPanel, not tooltip-only “Collapse chat history” names).@red-hat-developer-hub/backstage-plugin-intelligent-assistant; add@patternfly/react-stylesdependency for MCP table sort styling.Screen recording (after fix)
rhdhbugs_3733.mp4
Test plan
/intelligent-assistantfullscreen; confirm chat border, header divider spacing, and no stray footer<hr>above the input.yarn test --watchAll=falseinplugins/intelligent-assistant;APP_MODE=legacy yarn test:e2e(or targetedlightspeed.ui.test.ts) inworkspaces/intelligent-assistant.Checklist
Made with Cursor