Skip to content

feat(ui): add page title context#34

Merged
rowan-stein merged 2 commits into
mainfrom
noa/issue-33
Apr 8, 2026
Merged

feat(ui): add page title context#34
rowan-stein merged 2 commits into
mainfrom
noa/issue-33

Conversation

@casey-brooks

Copy link
Copy Markdown
Contributor

Summary

  • add PageTitleContext and useDocumentTitle hook for dynamic header titles
  • update org/agent pages to use page titles and remove top-level headings
  • align E2E selectors and add PageTitleContext unit coverage

Testing

  • npm run lint
  • npm run typecheck
  • npm test
  • npm run build

Closes #33

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary

  • added PageTitleContext/useDocumentTitle to drive the AppLayout header
  • removed page-level headings across org/agent pages in favor of the shared title
  • updated E2E selectors and added PageTitleContext unit coverage

Testing

  • npm run lint (no errors)
  • npm run typecheck
  • npm test (31 passed)
  • npm run build

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Testing

  • npm run lint (no errors)
  • npm run typecheck
  • npm test (31 passed, 0 failed, 0 skipped)
  • npm run build

@rowan-stein

Copy link
Copy Markdown
Contributor

CI Status

  • build — passed (lint, typecheck, tests, build)
  • e2e — passed (12m34s)
  • ⚠️ argos — 12 changed screenshots, waiting for approval

The Argos visual diffs are expected — we removed page-level headers from all pages, so every screenshot that included a heading will show a visual difference. These need manual approval in the Argos dashboard.

Requesting code review.

@noa-lucent noa-lucent 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.

Overall the PR is well-structured and consistent — the context split, hook pattern, and page-level changes all align with the spec and existing codebase conventions. Clean work.

Three items to address:

  1. [major] Missing useDocumentTitle unit test — the issue spec requires test case 4 covering mount-time title setting and updates on prop change.
  2. [minor] OrganizationDetailLayout retains a full getOrganization query whose data is no longer consumed — consider removing it.
  3. [minor] OrganizationMonitoringTab has a redundant nested space-y-6 div wrapper after heading removal — flatten it.

Comment thread src/__tests__/page-title-context.test.tsx
Comment thread src/pages/OrganizationDetailLayout.tsx Outdated
Comment thread src/pages/OrganizationMonitoringTab.tsx
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary

  • added useDocumentTitle hook coverage with rerender assertions
  • removed unused org detail query and simplified monitoring layout wrapper

Testing

  • npm run lint (no errors)
  • npm run typecheck
  • npm test (32 passed, 0 failed, 0 skipped)
  • npm run build

@noa-lucent noa-lucent 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.

All three items verified in code and resolved:

  1. useDocumentTitle test added — covers mount-time title setting and re-render updates via DocumentTitleProbe.
  2. OrganizationDetailLayout cleaned up — unused orgQuery, useQuery, useParams, and organizationsClient imports removed. Minimal wrapper now.
  3. OrganizationMonitoringTab flattened — redundant nested space-y-6 div eliminated.

PR fully resolves #33. Clean, consistent, well-tested. Good to merge.

@rowan-stein
rowan-stein merged commit 043243f into main Apr 8, 2026
2 of 3 checks passed
rowan-stein pushed a commit that referenced this pull request Apr 8, 2026
Follow-up to PR #34: merge search input and action button into a single row
(search left, button right) on all 14 list pages where they ended up on
separate rows after the header removal.

Closes #33
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.

Move page title to top bar, remove redundant headers from main area

3 participants