feat(web): standardize shared page layouts - #67
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (19)
📝 WalkthroughWalkthroughChangesShared UI modernization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/components/layouts/split-layout-components.tsx`:
- Around line 5-10: Update SplitRoot and the related SplitMain/aside layout
behavior to provide a responsive narrow-screen state before the fixed aside
forces the main pane to zero width. At the breakpoint covering the aside’s
minimum width plus the main pane requirement, stack the panes, collapse the
aside, or expose it through an accessible responsive drawer; ensure the primary
content remains reachable and the collapsed/drawer state has appropriate
accessible labeling and controls.
In `@web/src/components/patterns/data-table.tsx`:
- Around line 24-29: Update the `@tanstack/react-table` module augmentation by
importing RowData and constraining ColumnMeta’s TData generic to extend RowData,
while leaving TValue and the existing metadata properties unchanged.
In `@web/src/components/ui/breadcrumb.tsx`:
- Around line 52-60: Update BreadcrumbPage to remove the role="link" and
aria-disabled="true" attributes, while retaining aria-current="page" on the
span.
In `@web/src/features/sources/components/graph/graph-properties-panel.tsx`:
- Line 85: Position the relevant SplitLayout.Root/container context before the
absolute panel rendered by SplitLayout.Aside, ensuring the mobile right-3/top-3
overlay is contained within the canvas. Add a relative positioning context at
the appropriate parent level while preserving the existing lg:relative desktop
behavior and panel layout.
In `@web/src/features/sources/components/knowledge-graph-panel.tsx`:
- Around line 280-311: Update the onExpandEntity handler to catch fetchQuery
failures from exploreKnowledgeGraphOptions and surface them through toast.error,
preserving the existing success, authorization-refresh, and no-new-neighbors
flows. Ensure the async callback resolves after handling the error so the void
caller does not produce an unhandled rejection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2748b3b6-e3f3-4a1f-9271-832c1bcd9fe6
⛔ Files ignored due to path filters (7)
docs/conventions.mdis excluded by!docs/**docs/increments/completed/2026-07-26-shared-page-system/design.mdis excluded by!docs/**docs/increments/completed/2026-07-26-shared-page-system/plan.mdis excluded by!docs/**docs/roadmap.mdis excluded by!docs/**docs/specs/domains/asset-registry.mdis excluded by!docs/**docs/specs/domains/secure-knowledge-graph.mdis excluded by!docs/**web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (24)
web/package.jsonweb/src/components/layouts/page-layout-components.tsxweb/src/components/layouts/page-layout.tsweb/src/components/layouts/split-layout-components.tsxweb/src/components/layouts/split-layout.tsweb/src/components/patterns/content.tsxweb/src/components/patterns/data-table.tsxweb/src/components/patterns/empty-state.tsxweb/src/components/patterns/filter-bar.tsxweb/src/components/ui/breadcrumb.tsxweb/src/features/admin/components/admin-page.tsxweb/src/features/admin/components/admin-user-permissions-page.tsxweb/src/features/assets/components/asset-breadcrumb.tsxweb/src/features/assets/components/asset-catalog-page.tsxweb/src/features/assets/components/asset-detail-page.tsxweb/src/features/assets/components/asset-state.tsxweb/src/features/assets/components/governance-workspace-page.tsxweb/src/features/assets/components/pack-journey-page.tsxweb/src/features/mcp/components/mcp-connect-page.tsxweb/src/features/sources/components/graph/graph-properties-panel.tsxweb/src/features/sources/components/knowledge-graph-panel.tsxweb/src/features/sources/components/sources-page.tsxweb/src/features/sources/components/sources-table.tsxweb/src/styles/tokens.css
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CI Gate
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Record current behavior in architecture/specification documentation only after it exists in code; keep intended behavior in vision, roadmap, or an active increment, and do not duplicate state.
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, Gradle, React, Vite, Tailwind, or TypeScript APIs, consult current official documentation via Context7 and the projectorgmemory-*verification skills.
Before retrieval, AI, MCP, permission, upload, graph, or export work, readdocs/guidelines/agent-safety.md.
Never commit.envfiles, provider keys, tokens, or customer data.
Run the relevant verification gates fromdocs/guidelines/testing-harness.md; use a terminating clean test as the context gate, and do not treatbootRunas verification.
Files:
web/src/components/layouts/split-layout.tsweb/src/components/patterns/filter-bar.tsxweb/src/components/layouts/page-layout.tsweb/src/features/assets/components/asset-breadcrumb.tsxweb/package.jsonweb/src/components/patterns/empty-state.tsxweb/src/components/layouts/split-layout-components.tsxweb/src/components/patterns/content.tsxweb/src/features/admin/components/admin-user-permissions-page.tsxweb/src/features/assets/components/asset-state.tsxweb/src/features/sources/components/sources-table.tsxweb/src/components/layouts/page-layout-components.tsxweb/src/features/assets/components/asset-catalog-page.tsxweb/src/features/mcp/components/mcp-connect-page.tsxweb/src/components/ui/breadcrumb.tsxweb/src/features/assets/components/governance-workspace-page.tsxweb/src/components/patterns/data-table.tsxweb/src/features/assets/components/asset-detail-page.tsxweb/src/styles/tokens.cssweb/src/features/sources/components/sources-page.tsxweb/src/features/assets/components/pack-journey-page.tsxweb/src/features/sources/components/graph/graph-properties-panel.tsxweb/src/features/sources/components/knowledge-graph-panel.tsxweb/src/features/admin/components/admin-page.tsx
**/*.{ts,tsx,js,jsx,css,scss,html}
📄 CodeRabbit inference engine (CLAUDE.md)
For frontend files, run Oxlint, TypeScript typecheck, the production build, and browser tests when the UI flow matters; do not run JetBrains IDE inspection on TypeScript, TSX, or web configuration.
Files:
web/src/components/layouts/split-layout.tsweb/src/components/patterns/filter-bar.tsxweb/src/components/layouts/page-layout.tsweb/src/features/assets/components/asset-breadcrumb.tsxweb/src/components/patterns/empty-state.tsxweb/src/components/layouts/split-layout-components.tsxweb/src/components/patterns/content.tsxweb/src/features/admin/components/admin-user-permissions-page.tsxweb/src/features/assets/components/asset-state.tsxweb/src/features/sources/components/sources-table.tsxweb/src/components/layouts/page-layout-components.tsxweb/src/features/assets/components/asset-catalog-page.tsxweb/src/features/mcp/components/mcp-connect-page.tsxweb/src/components/ui/breadcrumb.tsxweb/src/features/assets/components/governance-workspace-page.tsxweb/src/components/patterns/data-table.tsxweb/src/features/assets/components/asset-detail-page.tsxweb/src/styles/tokens.cssweb/src/features/sources/components/sources-page.tsxweb/src/features/assets/components/pack-journey-page.tsxweb/src/features/sources/components/graph/graph-properties-panel.tsxweb/src/features/sources/components/knowledge-graph-panel.tsxweb/src/features/admin/components/admin-page.tsx
web/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
web/src/**/*.{ts,tsx}: OAuth access and refresh tokens must never enter browser JavaScript or
browser storage. Use the HttpOnly BFF session, CSRF-protected mutations,
generated Hey API data clients, accessible states, and both light and
dark themes. Handwritten transport is reserved for documented protocol
flows such as navigation redirects and streaming.
Files:
web/src/components/layouts/split-layout.tsweb/src/components/patterns/filter-bar.tsxweb/src/components/layouts/page-layout.tsweb/src/features/assets/components/asset-breadcrumb.tsxweb/src/components/patterns/empty-state.tsxweb/src/components/layouts/split-layout-components.tsxweb/src/components/patterns/content.tsxweb/src/features/admin/components/admin-user-permissions-page.tsxweb/src/features/assets/components/asset-state.tsxweb/src/features/sources/components/sources-table.tsxweb/src/components/layouts/page-layout-components.tsxweb/src/features/assets/components/asset-catalog-page.tsxweb/src/features/mcp/components/mcp-connect-page.tsxweb/src/components/ui/breadcrumb.tsxweb/src/features/assets/components/governance-workspace-page.tsxweb/src/components/patterns/data-table.tsxweb/src/features/assets/components/asset-detail-page.tsxweb/src/features/sources/components/sources-page.tsxweb/src/features/assets/components/pack-journey-page.tsxweb/src/features/sources/components/graph/graph-properties-panel.tsxweb/src/features/sources/components/knowledge-graph-panel.tsxweb/src/features/admin/components/admin-page.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: kl3inIT/OrgMemory
Timestamp: 2026-07-26T16:54:02.879Z
Learning: Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, or parity scope require an independent Claude Fable 5 architecture debate before implementation, recorded in the active increment or an ADR.
📚 Learning: 2026-07-23T03:36:09.053Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 16
File: web/src/features/admin/components/admin-mappings-page.tsx:190-211
Timestamp: 2026-07-23T03:36:09.053Z
Learning: In the OrgMemory admin UI, preserve the principal display ordering returned by the backend instead of re-sorting on the client. The server-owned order (e.g., unmapped views first, then deterministic ordering by source-system, connection, kind, and external-key as defined in SourcePrincipalAdminService#listPrincipals) must be used as-is to avoid mismatches with the server’s intended admin mappings/principals list.
Applied to files:
web/src/features/admin/components/admin-user-permissions-page.tsxweb/src/features/admin/components/admin-page.tsx
🔇 Additional comments (30)
web/src/features/sources/components/sources-page.tsx (5)
8-10: LGTM!Also applies to: 47-50
76-106: LGTM!
108-147: LGTM!
149-190: LGTM!
192-196: LGTM!Also applies to: 212-215, 237-241
web/src/features/sources/components/sources-table.tsx (2)
2-5: LGTM!Also applies to: 32-139
141-153: LGTM!web/src/features/sources/components/graph/graph-properties-panel.tsx (1)
16-16: LGTM!Also applies to: 122-122, 144-144, 197-197, 346-346, 590-591
web/src/features/sources/components/knowledge-graph-panel.tsx (3)
16-20: LGTM!Also applies to: 50-50, 117-208
227-266: LGTM!
491-493: LGTM!Also applies to: 545-548, 722-722
web/src/features/mcp/components/mcp-connect-page.tsx (1)
4-150: LGTM!Also applies to: 190-190, 199-201
web/src/components/layouts/page-layout-components.tsx (1)
6-146: LGTM!web/src/components/layouts/page-layout.ts (1)
1-19: LGTM!web/src/components/layouts/split-layout.ts (1)
1-7: LGTM!web/src/styles/tokens.css (1)
5-7: LGTM!Also applies to: 94-109, 211-211
web/package.json (1)
34-34: LGTM!web/src/components/patterns/content.tsx (1)
5-83: LGTM!web/src/components/patterns/empty-state.tsx (1)
5-38: LGTM!web/src/components/patterns/filter-bar.tsx (1)
5-34: LGTM!web/src/features/admin/components/admin-page.tsx (1)
4-6: LGTM!Also applies to: 29-32, 107-113, 122-130
web/src/features/admin/components/admin-user-permissions-page.tsx (1)
3-16: LGTM!Also applies to: 27-33, 90-116, 238-238
web/src/features/assets/components/asset-breadcrumb.tsx (1)
1-63: LGTM!web/src/features/assets/components/asset-catalog-page.tsx (2)
5-26: LGTM!Also applies to: 57-101
103-176: 🗄️ Data Integrity & IntegrationNo change needed. The
/assets/$assetIdroute definesvalidateSearchwithrelease?: string, so both explicitreleasevalues and an omitted{}search object are accepted.web/src/features/assets/components/asset-detail-page.tsx (2)
181-188: 🎯 Functional Correctness | ⚡ Quick winGuard against a blank page title.
title={release?.title ?? asset.draft?.title}can resolve toundefinedwhen an asset has neither a release nor a draft title, rendering an empty<h1>in the sharedPageHeader. A trivial fallback avoids the empty-header edge case.🩹 Proposed fix
<PageLayout.Header - title={release?.title ?? asset.draft?.title} + title={release?.title ?? asset.draft?.title ?? "Untitled asset"} description={release?.summary ?? asset.draft?.summary}
16-16: LGTM!Also applies to: 34-40, 52-52, 137-160, 189-254, 268-270, 311-311, 385-385, 459-462, 544-553, 690-690, 738-740
web/src/features/assets/components/asset-state.tsx (1)
3-18: LGTM!Also applies to: 29-46
web/src/features/assets/components/governance-workspace-page.tsx (1)
15-15: LGTM!Also applies to: 36-36, 67-121, 144-155, 535-537
web/src/features/assets/components/pack-journey-page.tsx (1)
14-22: LGTM!Also applies to: 65-224
kl3inIT
marked this pull request as ready for review
July 26, 2026 17:56
This was referenced Jul 27, 2026
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
PageLayoutcontract with narrow, standard, wide, full, and canvas variantsSplitLayout, shared content/filter/empty patterns, breadcrumbs, and a semanticDataTableWhy
The existing pages repeated width, gutter, scrolling, header, toolbar, and empty-state markup. This made the Knowledge graph inherit a centered document layout and allowed each feature to develop a different page dialect. The new contract applies the reusable layout approach observed in the local Onyx reference without copying its styling or introducing a new aesthetic.
Stack decisions
Verification
corepack pnpm -C web lintcorepack pnpm -C web typecheckcorepack pnpm -C web buildcorepack pnpm -C web test:e2e— 8 passedNotes
Summary by CodeRabbit