[Chakra exit · Phase 2 · PR 15] frontend: the topics pages on Registry components, plus a settings-dialog spec - #2647
Open
SpicyPete wants to merge 7 commits into
Open
[Chakra exit · Phase 2 · PR 15] frontend: the topics pages on Registry components, plus a settings-dialog spec#2647SpicyPete wants to merge 7 commits into
SpicyPete wants to merge 7 commits into
Conversation
These four outlive the enableNewTopicMessagesPage flag, so migrating
them is not throwaway work. Three of them are rendered by page areas
already marked done: MessagePreview paints the Value column of the logs
tables on transforms, connector-details and pipelines-details, and
renderEmptyIcon reaches the same three through expanded-message ->
message-headers. Those pages have carried Chakra at runtime since their
own migrations merged — the metric counts imports per file, and these
imports live under topics/.
tab-docu is the topic Documentation tab, which was never behind the
flag at all.
Chakra's colour props become semantic tokens: red.600 -> text-destructive,
blue.500 -> text-informative, gray.500 -> text-muted-foreground. The
empty-icon tooltip keeps its instant open (Chakra had openDelay={1};
the Registry would otherwise inherit Base UI's 600ms) and an exact
opacity-[0.66] rather than a bare opacity-66, which depends on v4's
dynamic scale.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four files that only the flag-off view reaches: two Chakra Modals
become Registry Dialogs, the glob-pattern Drawer becomes a Sheet, and
the preview-settings popover becomes a Registry Popover. Both dialogs
keep the PortalContainerProvider from the Chakra original so their
nested select and popover still portal inside the dialog's focus and
scroll lock.
Chakra's Checkbox took its label as a child and wired it; the Registry's
does not, so every box now has an explicit id + Label, or an aria-label
where the original had no visible text at all.
Removed rather than ported: a Popover with no trigger child in
preview-settings, which rendered nothing clickable and had no way to
open. Also removed the dead `rs.mock('@redpanda-data/ui')` block in
message-display.test.tsx — with these components off Chakra it mocked
nothing, and dropping it means those four tests now exercise the real
components instead of plain divs.
Adds a spec for the two dialogs and the sheet: four specs already drive
the legacy messages view, but none of them opened these, and a dialog
that fails to open is invisible to the type checker.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DialogContent renders its own close X with an aria-label of "Close", so a role query by name matches it as well as the footer button. Only the footer button carries the text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
SpicyPete
changed the title
SpicyPete
requested review from
a team,
c-julin,
datamali,
r-vasquez and
yougotashovel
and removed request for
a team
September 10, 2026 15:24
… they did The portal-container wrapper inside DialogContent was a plain block, so DialogBody's `flex-1` and its own scrolling were inert: tall content was clipped by the popup's max-height instead of scrolling, and the footer could be pushed out of view. The wrapper is now the flex column DialogBody expects. SheetContent is likewise not a flex column, so the drawer's header scrolled away with the body and SheetFooter's flex-col stretched the lone Close button across the panel. Made the content a column, scrolled only the body, and right-aligned the footer as the Chakra Drawer's was. Also dropped a redundant `w-full` on Empty, whose root already ships one, and the comment that claimed otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
noLeakedRender's sibling rule useTopLevelRegex fires on an inline regex in a locator; the reassign spec already keeps its matchers as module constants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four behaviour findings from the review of #2647: - the glob-pattern Sheet honoured the enclosing dialog's PortalContainerProvider, so it mounted inside DialogContent — which is transformed and `overflow-hidden`, making it both the containing block and the clipping ancestor for a fixed panel. The help was therefore sized and clipped to the dialog box rather than covering the viewport, as the Chakra Drawer did. `container` opts out. - `size="xl"` was not the like-for-like it looked like: Chakra's drawer scale maps `xl` to 56rem, the Sheet's to 36rem. The width is now set explicitly, with `size="full"` dropping the variant's own cap. - `fontSize` never sized the lucide warning and info icons, so master rendered them at 24 and `size={16}` shrank them by a third — in the messages table and in the transforms, connector-details and pipelines-details logs tables. - tab-docu's documentation button lost Chakra's `solid` fill to the Registry's neutral default. The spec grows a guard that the Sheet is not a descendant of the dialog; `toBeVisible` cannot see overflow clipping. It also stops locating dialogs by role: Base UI's Toast.Root is a role=dialog too, so `getByRole('dialog')` matched the messages tab's own "Searching..." toast whenever it was still up, and the spec failed roughly half the time on a strict-mode violation. Scoped by slot it passes 8/8 with --repeat-each=4. Both tests now delete the topic they create, as the eight sibling topic specs do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment-only. The spec preamble and the Sheet and portal-container notes had grown into rationale; each is now the constraint alone, with the reasoning left to the PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
r-vasquez
approved these changes
Sep 10, 2026
r-vasquez
left a comment
r-vasquez
left a comment
Contributor
There was a problem hiding this comment.
🚀
Tested locally and all loogs good.
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.
Takes
pages/topicsoff Chakra — all nine remaining files.mastergoes 28 → 19, leaving only the four security tabs (#2604) and PR 13's own five.Stacked conceptually after #2645 (rp-connect) but independent of it: disjoint files, opened off
master, merges in either order.Why this is not the throwaway work the plan assumed
The tracker has said for a week that these nine "go with the legacy view once
enableNewTopicMessagesPageflips". A reverse-dependency check says otherwise.Tab.Messagesis not a self-contained legacy view — it is a shared library, with eight modules imported from outside the directory, including by the new messages page that replaces it.Five of the nine survive the flag flip, and three are rendered by page areas already marked done:
tab-docu.tsxtopic-details.tsx— the Documentation tab, never behind the flagmessage-display/message-preview.tsxcommon/empty-badge.tsxmessage-previewcommon/empty-icon.tsxexpanded-message → message-headers, same three pagespreview-settings.tsxmessages/table/message-cells.tsximportsgetPreviewTagsSo
MessagePreviewhas been painting Chakra<Flex>/<Text>— withred.600,blue.500,gray.500— in the Value column of three logs tables since their own migrations merged. The importer metric could not see it: the imports live undertopics/, so they were booked against the topics bucket while those pages counted as clean. Worth stating once: "0 importers" is the right gate for removing the dependency, and the wrong one for "is this page migrated".The remaining four (
message-key-preview,modals/column-settings,modals/preview-fields-modal,preview-settings/pattern-help-drawer) really are legacy-only. I migrated them anyway — PR 13 cannot merge while anything imports the package, and the flag flip has no date. That work is deliberately spent so the flip stops being a gate.Reviewer's guide
red.600→text-destructive,blue.500→text-informative,gray.500→text-muted-foreground. These werecolor=props on Chakra components, so they worked; in Registry they need semantic classes.Tooltip→ compoundempty-iconkeeps its instant open — Chakra hadopenDelay={1}, and the Registry would otherwise inherit Base UI's 600 ms.opacity-[0.66]opacity-66. A bare number depends on v4's dynamic scale; an arbitrary value is guaranteed to emit, and a silently-inert opacity is the exact trap this series keeps hitting.Modal→DialogPortalContainerProviderfrom the Chakra original, so the nested select and popover still portal inside the dialog's focus and scroll lock rather than to the body.Drawer→Sheetplacement="right" size="xl"→side="right" size="xl"; the body scrolls viaoverflow-y-autoon the content.Checkboxid+Label, or anaria-labelwhere the original had no visible text at all — the pattern-enable box inpreview-settingshad none.EmptyEmpty description={title}→Empty+EmptyHeader+EmptyTitle, withw-fullbecause the Registry root has no width of its own.Removed rather than ported
Popoverwith no trigger child inpreview-settings(content={globHelp} trigger="click", self-closing). It rendered nothing clickable and had no way to open — the adjacentPatternHelpDrawerprovides the real glob-pattern link. Porting it faithfully would have meant a Registry Popover with an empty trigger.rs.mock('@redpanda-data/ui')block inmessage-display.test.tsx. With these components off Chakra it mocked nothing. Dropping it means those four tests now exercise the real components instead of plaindivs — same four passing, strictly better coverage.Coverage
Four specs already drive the legacy messages view (filtering, production, timestamps, navigation), so
MessagePreviewandMessageKeyPreviewhave real regression cover. None of them opened the three settings surfaces, which are the riskiest part of this diff — two dialog conversions and a drawer.topic-messages-settings-dialogs.spec.tscovers them: the column-settings dialog opens, its checkboxes resolve by name (the contract Chakra used to provide), it closes; and the preview-fields dialog opens the glob-pattern sheet nested inside it. Verified green against a live container. It caught one real thing while being written:DialogContentrenders its own close X witharia-label="Close", so a role-by-name query matches two buttons.Gates
type:checkclean ·lint:checkclean on all 11 changed files · 993 unit · 1425 integration · the new e2e spec passes in theconsolevariant.Flagged, not fixed
preview-settings.tsxis still imported by the new page for one function.getPreviewTagsis a pure helper sitting in a 543-line component module. Extracting it would let the new page stop importing a component module entirely; left alone here because the module is now Registry-based either way, so it is hygiene rather than a dependency problem.🤖 Generated with Claude Code