Skip to content

fix(ui): restore consistent page rhythm - #72

Merged
kl3inIT merged 2 commits into
mainfrom
agent/ui-layout-rhythm-fix
Jul 27, 2026
Merged

fix(ui): restore consistent page rhythm#72
kl3inIT merged 2 commits into
mainfrom
agent/ui-layout-rhythm-fix

Conversation

@kl3inIT

@kl3inIT kl3inIT commented Jul 27, 2026

Copy link
Copy Markdown
Owner

What changed

  • make PageLayout.Body own the ordinary page top rhythm and apply the Header → Body composition consistently
  • separate Asset filters/results, Admin sections, Asset detail/governance/journey content, and MCP setup surfaces
  • remove the hard-coded MCP Available/protocol badges and replace the oversized security callout with compact factual guidance
  • document the Onyx-backed composition decision and add focused Vitest/Playwright regressions

Root cause

The shared page foundation introduced PageLayout.Body, but feature pages continued rendering independent surfaces directly under PageLayout.Root. With no root-level content gap, filters, cards, and searches visually touched even though they were unrelated sections.

User impact

Pages now use consistent vertical rhythm without changing their business behavior. MCP onboarding no longer implies a live health status that the UI has not checked.

Validation

  • corepack pnpm -C web typecheck
  • corepack pnpm -C web lint
  • corepack pnpm -C web test:unit — 5 passed
  • corepack pnpm -C web build
  • corepack pnpm -C web test:e2e — 8 passed
  • git diff --check

Summary by CodeRabbit

  • UI Improvements

    • Standardized page spacing and content regions across administration, asset, governance, and pack journey screens.
    • Improved asset catalog, detail, loading, and error page layouts.
    • Simplified the MCP connection page with a clearer permissions callout and streamlined presentation.
  • Bug Fixes

    • Improved spacing between asset filters and results.
  • Tests

    • Added coverage for page body layout, asset spacing, and updated MCP onboarding expectations.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kl3inIT, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 46528fe2-117f-47df-9b97-8c1521cc0c60

📥 Commits

Reviewing files that changed from the base of the PR and between db92fac and 1985e05.

📒 Files selected for processing (1)
  • web/test/e2e/mcp-connect.spec.ts
📝 Walkthrough

Walkthrough

The change standardizes page content placement through PageLayout.Header and PageLayout.Body, adds default body spacing, and updates related component and E2E tests. The MCP connection page also receives a simplified permissions presentation and revised onboarding assertions.

Changes

Page layout standardization

Layer / File(s) Summary
Shared PageBody spacing and coverage
web/src/components/layouts/page-layout-components.tsx, web/src/components/layouts/page-layout.test.tsx
Adds pt-6 to PageBody and verifies the body slot and spacing class.
Admin page body migration
web/src/features/admin/components/admin-page.tsx, web/src/features/admin/components/admin-user-permissions-page.tsx
Places admin content, permission cards, and AccessInspector inside PageLayout.Body.
Asset page layout migration
web/src/features/assets/components/*
Moves catalog, detail, loading, error, governance, and pack journey content into shared header and body regions.
Asset spacing validation
web/test/e2e/asset-registry-golden-poc.spec.ts
Checks that the visible assets region begins at least 16px below the filter bar.

MCP onboarding update

Layer / File(s) Summary
MCP connection interface
web/src/features/mcp/components/mcp-connect-page.tsx
Simplifies the MCP card, removes SecurityPoint, and replaces the capability section with a permissions callout.
MCP onboarding assertions
web/test/e2e/mcp-connect.spec.ts
Updates wording checks and verifies removed capability text is absent.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: normalizing page spacing and layout rhythm across UI pages.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/ui-layout-rhythm-fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/test/e2e/mcp-connect.spec.ts`:
- Around line 42-46: Strengthen the visibility assertion in the test around the
OrgMemory permissions message to explicitly require the non-mutation text
“cannot publish or modify them,” rather than matching only the opening sentence.
Keep the assertion scoped to the same page content and preserve the existing
visibility check.
🪄 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: 640232d8-c9a5-4e00-bf7c-05bea87d9630

📥 Commits

Reviewing files that changed from the base of the PR and between ccf591f and db92fac.

⛔ Files ignored due to path filters (2)
  • docs/increments/active/2026-07-27-page-rhythm-follow-up/design.md is excluded by !docs/**
  • docs/increments/active/2026-07-27-page-rhythm-follow-up/plan.md is excluded by !docs/**
📒 Files selected for processing (12)
  • web/src/components/layouts/page-layout-components.tsx
  • web/src/components/layouts/page-layout.test.tsx
  • web/src/features/admin/components/admin-page.tsx
  • web/src/features/admin/components/admin-user-permissions-page.tsx
  • web/src/features/assets/components/asset-catalog-page.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/assets/components/asset-state.tsx
  • web/src/features/assets/components/governance-workspace-page.tsx
  • web/src/features/assets/components/pack-journey-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • web/test/e2e/mcp-connect.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Web · Node 24
🧰 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 project orgmemory-* verification skills.
Before retrieval, AI, MCP, permission, upload, graph, or export work, read docs/guidelines/agent-safety.md.
Never commit .env files, provider keys, tokens, or customer data.
Run the relevant verification gates from docs/guidelines/testing-harness.md; use a terminating clean test as the context gate, and do not treat bootRun as verification.

Files:

  • web/src/components/layouts/page-layout-components.tsx
  • web/src/components/layouts/page-layout.test.tsx
  • web/src/features/admin/components/admin-page.tsx
  • web/src/features/assets/components/governance-workspace-page.tsx
  • web/src/features/assets/components/asset-state.tsx
  • web/test/e2e/mcp-connect.spec.ts
  • web/src/features/assets/components/asset-catalog-page.tsx
  • web/src/features/admin/components/admin-user-permissions-page.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • web/src/features/assets/components/pack-journey-page.tsx
  • web/src/features/mcp/components/mcp-connect-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/page-layout-components.tsx
  • web/src/components/layouts/page-layout.test.tsx
  • web/src/features/admin/components/admin-page.tsx
  • web/src/features/assets/components/governance-workspace-page.tsx
  • web/src/features/assets/components/asset-state.tsx
  • web/test/e2e/mcp-connect.spec.ts
  • web/src/features/assets/components/asset-catalog-page.tsx
  • web/src/features/admin/components/admin-user-permissions-page.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/test/e2e/asset-registry-golden-poc.spec.ts
  • web/src/features/assets/components/pack-journey-page.tsx
  • web/src/features/mcp/components/mcp-connect-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/page-layout-components.tsx
  • web/src/components/layouts/page-layout.test.tsx
  • web/src/features/admin/components/admin-page.tsx
  • web/src/features/assets/components/governance-workspace-page.tsx
  • web/src/features/assets/components/asset-state.tsx
  • web/src/features/assets/components/asset-catalog-page.tsx
  • web/src/features/admin/components/admin-user-permissions-page.tsx
  • web/src/features/assets/components/asset-detail-page.tsx
  • web/src/features/assets/components/pack-journey-page.tsx
  • web/src/features/mcp/components/mcp-connect-page.tsx
🧠 Learnings (1)
📚 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-page.tsx
  • web/src/features/admin/components/admin-user-permissions-page.tsx
🔇 Additional comments (12)
web/src/features/mcp/components/mcp-connect-page.tsx (1)

1-1: LGTM!

Also applies to: 24-49, 77-77, 93-93, 116-126

web/test/e2e/mcp-connect.spec.ts (1)

34-41: LGTM!

Also applies to: 65-66

web/src/components/layouts/page-layout-components.tsx (1)

123-131: LGTM!

web/src/components/layouts/page-layout.test.tsx (1)

38-53: LGTM!

web/src/features/admin/components/admin-page.tsx (1)

29-33: LGTM!

web/src/features/admin/components/admin-user-permissions-page.tsx (1)

118-239: LGTM!

web/src/features/assets/components/asset-catalog-page.tsx (1)

58-170: LGTM!

web/src/features/assets/components/asset-detail-page.tsx (1)

143-161: LGTM!

Also applies to: 184-251

web/src/features/assets/components/asset-state.tsx (1)

11-19: LGTM!

Also applies to: 33-49

web/src/features/assets/components/governance-workspace-page.tsx (1)

103-122: LGTM!

web/src/features/assets/components/pack-journey-page.tsx (1)

142-227: LGTM!

web/test/e2e/asset-registry-golden-poc.spec.ts (1)

55-61: LGTM!

Comment thread web/test/e2e/mcp-connect.spec.ts
@kl3inIT
kl3inIT merged commit 6509de2 into main Jul 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant