feat(console): adopt react 19.3 transitions and fragment refs - #2646
feat(console): adopt react 19.3 transitions and fragment refs#2646malinskibeniamin wants to merge 1 commit into
Conversation
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
malinskibeniamin
left a comment
malinskibeniamin
left a comment
There was a problem hiding this comment.
Automated /review: 4 finding(s).
| ::view-transition-group(root), | ||
| ::view-transition-old(root), | ||
| ::view-transition-new(root), | ||
| ::view-transition-group(.console-content-reveal), |
There was a problem hiding this comment.
Priority: P1
Reduced-motion opt-out never applies — invalid view-transition selector
::view-transition-group(.console-content-reveal) is not valid CSS. The argument grammar for these pseudo-elements is <pt-name-selector> (* or a <custom-ident>) optionally followed by a class selector, so a view-transition class must be written as *.console-content-reveal. A bare .console-content-reveal fails to parse, and because an invalid selector invalidates the entire selector list, the root opt-outs on lines 3-5 are dropped too.
Consequence: under prefers-reduced-motion: reduce the whole rule is discarded, so users who asked for reduced motion still get the full-page cross-fade every time a lazy editor resolves — the exact case this file exists to suppress.
Correction:
@media (prefers-reduced-motion: reduce) {
::view-transition-group(*.console-content-reveal),
::view-transition-old(*.console-content-reveal),
::view-transition-new(*.console-content-reveal) {
animation: none;
}
::view-transition-group(root),
::view-transition-old(root),
::view-transition-new(root) {
animation: none;
}
}Splitting the class rule from the root rule also keeps one bad selector from taking the other down in future edits.
Verify: load a page with a Monaco editor in Chrome with reduced motion forced (DevTools → Rendering → Emulate CSS prefers-reduced-motion: reduce) and confirm no cross-fade; DevTools Styles should show the rule matching rather than greyed out as invalid.
Automated /review.
| function ExpandedMessageActions({ onCollapse, onClose }: { onCollapse: () => void; onClose: () => void }) { | ||
| const actionsRef = useRef<FragmentInstance>(null); | ||
| useEffect(function focusExpandedActions() { | ||
| actionsRef.current?.focus({ preventScroll: true }); |
There was a problem hiding this comment.
Priority: P2
Focus is moved into the sheet but never restored when it unmounts
focusExpandedActions pulls focus onto the collapse button on mount, but every exit path unmounts this subtree: Escape and the collapse button flip expanded to false (topic-messages-view.tsx:529/551 swap presentations), and Close unmounts the panel entirely. Focus then falls back to document.body, so keyboard users lose their row position and restart the tab order from the top of the page. Because detailExpanded is restored from persisted state (topic-messages-view.tsx:79), simply clicking a row can also yank focus out of the table with no way back.
Correction: capture document.activeElement before focusing (or have the parent own the trigger element) and restore it in the effect cleanup, guarding that the saved element is still connected:
useEffect(function focusExpandedActions() {
const previous = document.activeElement as HTMLElement | null;
actionsRef.current?.focus({ preventScroll: true });
return () => {
if (previous?.isConnected) previous.focus({ preventScroll: true });
};
}, []);Verify: extend the new test to assert focus destination after Escape, Collapse, and Close (render the parent so the docked panel/table remains mounted), then tab-navigate the messages page manually to confirm focus returns to the originating control.
Automated /review.
| /** Reveal lazy content without animating its initial fallback or cached content. */ | ||
| export function LoadingBoundary({ children, fallback }: { children: ReactNode; fallback: ReactNode }) { | ||
| return ( | ||
| <ViewTransition default="none" update="console-content-reveal"> |
There was a problem hiding this comment.
Priority: P2
A page-wide view transition fires whenever any lazy editor resolves
<ViewTransition> drives a document-level transition, so the browser snapshots and cross-fades the root group as well — the CSS comment on loading-boundary.css:2 acknowledges this. Every Monaco/YAML editor reveal (kowl-editor.tsx:65, kowl-editor.tsx:82, pipelines-yaml-editor.tsx:109) therefore animates the entire viewport, not just the editor slot, and the page is non-interactive for the duration. Outside prefers-reduced-motion there is no opt-out at all, and with the selector defect above there is none inside it either.
Correction: scope the reveal so the root snapshot does not animate for all users — add an unconditional ::view-transition-group(root) { animation: none; } (or animation-duration: 0s) alongside the named class, keeping the reduced-motion block only for the named group. Alternatively gate the wrapper behind a matchMedia('(prefers-reduced-motion: reduce)') check and render a plain <Suspense> when motion is unwanted.
Verify: open the topic message detail value section in Chrome, record a performance trace during editor load, and confirm no full-viewport cross-fade and no input delay while the transition runs.
Automated /review.
| expect(screen.getByRole('button', { name: 'Collapse back to panel' })).toHaveFocus(); | ||
|
|
||
| await user.tab(); | ||
| const close = screen.getByRole('button', { name: 'Close' }); |
There was a problem hiding this comment.
Priority: P3
The "does not steal focus when the message changes" assertion cannot fail
focusExpandedActions has an empty dependency array and ExpandedMessageActions is not keyed by message, so a rerender with a new offset can never re-run the effect — this assertion is green regardless of the behaviour it claims to protect. The regression it is meant to guard (re-focusing when the selected record changes) would only appear if the subtree remounted, for example if the header were keyed on msg.offset or the panel were remounted by the parent.
Correction: drive the change through the real entrypoint instead — render the parent view (or toggle expanded off and on) and assert focus placement across that cycle, so a keyed remount or a dependency added to the effect is caught.
Verify: temporarily add key={msg.offset} to ExpandedMessageActions locally; the test should go RED. It currently stays green.
Automated /review.
1 participant
Summary
Impact
Keyboard users land on the expanded message's controls instead of losing focus to the document. Browser checks confirm animated loading reveals normally, zero CSS transition animations under reduced motion, and working close/reopen behavior. No performance claim.
React 19.3 feature coverage
Dependency upgrade path
Stable minor: React/DOM 19.2.7 → 19.3.0; @types/react 19.2.17 → 19.3.0; @types/react-dom 19.2.3 → 19.3.0; scheduler 0.27.0 → 0.28.0.
Requested exact release is less than the repository's 72-hour cooling window: used a one-shot
--minimum-release-age=0for the four requested React packages. Repository bunfig policy unchanged; lifecycle scripts disabled. Clean frozen install passed. No arbitrary coupled package churn; existing wrapper peers accept React 19. @monaco-editor/react 4.7.0 remains the latest stable release.React 18 Cloud host isolation is unchanged (
shared: {}and separate React roots).Sources: https://react.dev/blog/2026/09/09/react-19-3, https://react.dev/reference/react/ViewTransition, https://react.dev/reference/react/Fragment
Verification
bun run type:check,bun run build,bun run test:ci(993 unit + 1,429 integration + 1 federation tests).bun install --frozen-lockfile --ignore-scripts; lockfile diff limited to React ecosystem.lint:fixscript does not exist, so ran the project'sbun run lintequivalent.Draft publication limitations
User approved draft publication after disclosure of incomplete full-stack dogfood, the known upstream diff error, and missing uploaded before/after screenshot evidence. These remain verification limitations, not passing checks. Local evidence and scripts are under
.context/react-evidence/and.context/react-*.log; these are not reviewer-accessible URLs.Cleanup caveat: test network
1b3544115c222b3a307a8ff082a3bfa22b993a6ac6af66750845d2c76d112c54could not be checked/removed while Docker Desktop was unavailable. The interrupted test process is stopped.