Skip to content

docs: scope the page+hook rule to data/list pages - #131

Merged
lopatnov merged 1 commit into
mainfrom
docs/clarify-page-hook-form-scope
Sep 5, 2026
Merged

docs: scope the page+hook rule to data/list pages#131
lopatnov merged 1 commit into
mainfrom
docs/clarify-page-hook-form-scope

Conversation

@lopatnov

@lopatnov lopatnov commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Clarifies that the page = layout + useXxx hook rule in CLAUDE.md is scoped to data/list pages (loading/error/empty/data states), not every page
  • Notes that the four auth form pages (LoginPage, RegisterPage, ForgotPasswordPage, ResetPasswordPage) already use a second, consistent pattern — a one-shot gRPC call directly in the react-hook-form submit handler — so this isn't drift to "fix"

Related issue

Found while reviewing this cycle's architect refactor-analysis output: those four pages call authClient.* directly, which reads as a page+hook rule violation on a literal reading of CLAUDE.md. Investigation showed it's a deliberate, internally consistent second pattern (a dedicated hook for one gRPC call in a form submit handler would be pure ceremony), not something to refactor. Documenting the scope prevents a future pass from "fixing" four working pages for no benefit.

Type of change

  • Documentation update

Checklist

  • dotnet build --configuration Release passes with 0 errors (unaffected by this change)
  • cd src/pressmark-web && npm run build passes with 0 TypeScript errors (unaffected by this change)
  • New behavior is covered by tests — documentation-only change, no behavior change
  • All UI strings use t('ns:key') — no strings touched

Notes for reviewers

Doc-only change, no code touched. Part of the bi-weekly /maintain cycle (2026-09-04/05).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg


Generated by Claude Code

Summary by CodeRabbit

  • Documentation
    • Added frontend guidance for choosing patterns when building list, state, and simple form pages.

Auth form pages (Login/Register/ForgotPassword/ResetPassword) call the gRPC
client directly from their react-hook-form submit handler instead of going
through a dedicated hook. That's a second, already-consistent pattern for
one-shot form submissions, not drift from the page+hook rule — clarify the
rule's scope so it isn't mistaken for something to "fix" later.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 6b7807ec-8b2f-4f6d-9dbc-e6ce7a9fb2af

📥 Commits

Reviewing files that changed from the base of the PR and between 73ed79b and 30df235.

📒 Files selected for processing (1)
  • CLAUDE.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The frontend guidance now distinguishes list/state pages that use a page-plus-hook pattern from simple form pages that may submit one gRPC call directly.

Changes

Frontend guidance

Layer / File(s) Summary
Submission pattern guidance
CLAUDE.md
Documents page-plus-hook usage for list/state pages. Allows LoginPage, RegisterPage, ForgotPasswordPage, and ResetPasswordPage to submit one gRPC call directly without a dedicated hook.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 30df2

This updates frontend documentation to clarify when direct form submissions are appropriate, without changing application behavior or runtime dependencies.

Suggested reviewers: claude

Poem

A rabbit reads the frontend guide
List pages keep their hooks beside
Forms may call gRPC once
Four pages hop, then they are done
Clear rules help the burrow glide

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main documentation change: narrowing the page-and-hook rule to data and list pages.
Description check ✅ Passed The description includes all required sections, explains the documentation change, identifies the intentional auth form pattern, and records the validation results. The Related issue section does not …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 docs/clarify-page-hook-form-scope

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.

@gitar-bot

gitar-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Clarifies that the page+hook rule in CLAUDE.md applies to data/list pages, not all pages, and documents the separate pattern used by the four auth form pages. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@sonarqubecloud

sonarqubecloud Bot commented Sep 5, 2026

Copy link
Copy Markdown

@lopatnov
lopatnov marked this pull request as ready for review September 5, 2026 07:28
@lopatnov
lopatnov merged commit 9efbfd0 into main Sep 5, 2026
15 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.

2 participants