refactor(flux): migrate Flux details panel to bui (@backstage/ui)#1984
Merged
Conversation
Migrate the Flux details panel and the full ResourceCard tree from legacy @material-ui/core + makeStyles to bui (@backstage/ui). Resource cards are now collapsible Accordions (expanded by default) with a bold heading, status/kind/ namespace badges, and the not-ready background tint preserved. Per-kind color coding is kept on the resource-type badge, and the shared tree-view node keeps its original appearance (panel-only styling is gated behind an `emphasized` flag). Adds render tests for Details, ResourceCard, and CopyCommandMenu. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
marians
force-pushed
the
refactor/flux-details-panel-bui
branch
from
July 23, 2026 04:57
c94cf5b to
f7ff8ef
Compare
marians
commented
Jul 23, 2026
| <Accordion | ||
| id={`${kind}-${namespace ?? ''}-${name}`} | ||
| className={classes.accordion} | ||
| defaultExpanded |
Member
Author
There was a problem hiding this comment.
The card is an uncontrolled Accordion (defaultExpanded, no isExpanded), and defaultExpanded only applies on mount. DetailsPane/FluxResourceDetails/Details render this card in place without a key tied to the resource identity, so navigating between two resources of the same kind (e.g. Kustomization → Kustomization) keeps the same Accordion instance mounted. If the user collapsed the previous card, the new resources card stays collapsed instead of expanding by default. Consider keying the card (or Details/the detail component) by the resource ref so it re-mounts and re-applies defaultExpanded.
Member
Author
There was a problem hiding this comment.
Fixed in 31d8654 — the Accordion is now keyed by the resource identity (kind-namespace-name-cluster), so it re-mounts and re-applies defaultExpanded when the panel switches to a different resource of the same kind. Verified live: collapsed a HelmRelease card, navigated to another HelmRelease, and the new card came up expanded again.
marians
commented
Jul 23, 2026
| const color = inactive ? 'secondary' : 'primary'; | ||
| // The details panel (emphasized) gets a larger, bold heading; the tree-view | ||
| // node keeps its original, smaller size until it is migrated separately. | ||
| const as = emphasized ? 'h4' : 'div'; |
Member
Author
There was a problem hiding this comment.
In emphasized mode ResourceHeading renders <Text as="h4">, and this heading is placed inside AccordionTrigger, which (react-aria Disclosure) wraps its children in a <button> nested in a <Heading>. That yields <h3><button>…<h4>name</h4>…</button></h3> — a heading nested in a button nested in a heading. This is invalid HTML (a <button> may only contain phrasing content, and headings should not nest) and can confuse screen readers. Browsers tolerate it, but consider as="div" (or a non-heading element) for the emphasized title inside the trigger.
Member
Author
There was a problem hiding this comment.
Fixed in 31d8654 — the emphasized title now renders as a div (keeping the bold weight for the visual heading look) instead of h4, so there is no longer a heading nested inside the trigger <button>. Verified in the a11y tree that the AccordionTrigger button contains no heading elements.
marians
commented
Jul 23, 2026
| <TerminalIcon fontSize="small" /> | ||
| ) | ||
| } | ||
| aria-label="Copy CLI command" |
Member
Author
There was a problem hiding this comment.
The copy button lost its tooltip in the migration. The old code wrapped the button in a MUI <Tooltip title="Copy CLI command">; now there is only aria-label, which is not shown on hover. ButtonIcon does not render a visible tooltip from aria-label on its own, so sighted users lose the affordance label. Consider wrapping in bui TooltipTrigger/Tooltip.
Member
Author
There was a problem hiding this comment.
Fixed in 31d8654 — wrapped the ButtonIcon in bui TooltipTrigger/Tooltip. Verified live: hovering shows the "Copy CLI command" tooltip and the button now has a matching aria-describedby, and opening the menu still works with TooltipTrigger nested inside MenuTrigger.
marians
commented
Jul 23, 2026
| fixedKeyColumnWidth="120px" | ||
| /> | ||
| <Accordion | ||
| id={`${kind}-${namespace ?? ''}-${name}`} |
Member
Author
There was a problem hiding this comment.
The Accordion id is derived from kind/namespace/name only (not cluster). Each ResourceCard is a standalone Disclosure whose id drives the panel id and the trigger aria-controls. If the same kind+namespace+name identity appears in two cards on one panel (e.g. the primary "This X" card and a related/dependency card referring to the same object), the DOM ids collide — invalid HTML and mis-wired aria-controls. Including cluster (already part of the React key used for dependency cards) would make it unique.
Member
Author
There was a problem hiding this comment.
Fixed in 31d8654 — the Accordion id now includes cluster (kind-namespace-name-cluster), matching the React key already used for dependency cards, so the ids stay unique even when the same kind+namespace+name identity appears in more than one card on a panel.
- Restore the copy button's tooltip ("Copy CLI command") via bui
TooltipTrigger/Tooltip; aria-label alone was not shown on hover.
- Render the emphasized card title as a `div` instead of `h4`: it lives inside
the AccordionTrigger's <button> (wrapped in a react-aria heading), and a
heading nested in a button nested in a heading is invalid HTML.
- Include the cluster in the Accordion id so it stays unique when the same
kind+namespace+name identity appears in more than one card on a panel.
- Key the Accordion by the resource identity so it re-mounts and re-applies
`defaultExpanded` when the panel switches between resources of the same kind.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The node-build job runs `ci:verify` = tsc (--max-old-space-size=6144), lint:all, and test:all --coverage (131 suites) concurrently, which peaked near the xlarge 16 GB ceiling and OOM-killed the job intermittently (fast failure, no step logs, green locally). Bump the resource_class from xlarge to 2xlarge (32 GB) for headroom. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2xlarge is not available on this org's CircleCI plan (xlarge is the Docker ceiling), so the generated config was rejected at validation and the build workflow failed in ~2s. Restore xlarge to keep the pipeline runnable.
node-build runs tsc (--max-old-space-size=6144), lint:all, and test:all --coverage concurrently. On xlarge (16 GB) the combined peak reached the ceiling and OOM-killed the job intermittently (fast failure, no step logs, green locally). 2xlarge is not available on this org's CircleCI plan, so trim the Jest footprint instead: --maxWorkers 50%->40% (fewer concurrent workers on the 8-vCPU box) and --workerIdleMemoryLimit 1536MB->1024MB. Kept percentage- based so local full-test runs on larger machines are not crippled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JS Dependency Audit0 added · 0 removed · 207 total (0 vs base) Projects audited
No change in vulnerabilities compared to the base branch. Full current vulnerability list (207)
|
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
1 participant
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.
What does this PR do?
Migrates the Flux details panel — the body rendered inside
DetailsPanewhen a resource is selected in the tree/list view — and the fullResourceCardcomponent tree from legacy@material-ui/core+makeStylesto bui (@backstage/ui). This is the first Flux → bui PR.Section,Details, and the Kustomization/HelmRelease/Repository/ImageAutomation detail components) on buiFlex/Box/Text.ResourceCardtree (ResourceCard/ResourceWrapper,ResourceInfo,ResourceHeading,ResourceStatus,ResourceChips,ResourceMetadata,CopyCommandMenu).CopyCommandMenunow uses the buiMenuTrigger/Menuwith aButtonIcontrigger.Accordions (expanded by default): the name/kind/status row is the trigger, metadata/actions are the revealable panel. The card keeps its rounded corners and status-colored background (including the not-ready tint), the resource name is a bold heading, the status and type/namespace indicators are buiBadges, and the caret is aligned top-right.makeStyles, as bui has no equivalent tokens.What is the effect of this change to users?
Visual refresh of the Flux details panel; no behavioural change. Because
ResourceWrapper/ResourceInfoare shared with the tree-view node, panel-only styling is gated behind anemphasizedflag so the tree view keeps its original appearance (its full migration is a follow-up PR).How does it look like?
Verified live in the running app (light + dark) via DevTools. Collapsible cards with rounded corners, not-ready rose tint, bold heading, color-coded type badge, and neutral namespace/status badges.
Any background context you can provide?
First step of the incremental Flux → bui migration.
DetailsPaneshell (inui-react) and the tree-view node are out of scope here.Do the docs need to be updated?
No.
Should this change be mentioned in the release notes?