Skip to content

fix(series): render movement collection heading - #628

Merged
zacjones93 merged 32 commits into
codex/settings-athlete-form-context-fixfrom
codex/series-event-movements-form-context-fix
Jul 15, 2026
Merged

fix(series): render movement collection heading#628
zacjones93 merged 32 commits into
codex/settings-athlete-form-context-fixfrom
codex/series-event-movements-form-context-fix

Conversation

@zacjones93

@zacjones93 zacjones93 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • render the parent-event sub-event Movements collection with a semantic heading instead of the React Hook Form-only FormLabel
  • add focused route coverage for selected and empty movement states
  • lock the existing sub-event update payload with a regression test
  • document the sub-event movement editing contract in LAT

Root cause

SubEventForm rendered FormLabel directly under the form without a FormField provider. Parent series events therefore threw useFormField should be used within <FormField> before their active sub-event editor could render.

Scope

The production change is one route-local element replacement. Shared Form primitives, schemas, server functions, controller behavior, and update payloads are unchanged.

Verification

  • pre-fix focused test: 3/3 failed with the exact form-context error
  • post-fix focused test: 3/3 passed
  • full wodsmith-start suite: 2,945/2,945 passed
  • page coverage validation: 29/29 passed
  • wodsmith-start typecheck passed
  • repository pre-push lint and typecheck passed
  • UI inventory and page coverage ledgers are current
  • route Biome lint passed
  • lat check and git diff --check passed
  • GitNexus change detection: low risk, zero affected processes

Summary by cubic

Fix crash in the parent series sub-event editor by rendering the Movements collection as a semantic h2. Adds shared @repo/ui/field, @repo/ui/empty-state, and @repo/ui/metric primitives and applies them across Crew/Start to improve accessibility, responsive nav, and metric/empty-state composition with tests and docs.

  • Bug Fixes
    • Replace FormLabel with <h2> for Movements and use FieldGroup in related editors to remove form-context errors while keeping proper legends and headings.
    • Keep public nav within the mobile viewport and correct organizer/admin semantics (named nav landmarks, panel headings, single dashboard h1); demo seeding now derives dates from the event timezone.

Written for commit d6316c1. Summary will update on new commits.

Review in cubic

@zacjones93

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bd1f3d38-9390-4c65-b151-1497433ecb05

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/series-event-movements-form-context-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.

@zacjones93

Copy link
Copy Markdown
Contributor Author

Independent review receipt for exact head 8c62a8e

Finding:

  • [P2 accessibility] The new h3 skips directly from the page h1 because Parent Event Settings and Sub-Events use CardTitle, which renders a div, and the active tab has no h2. That makes Movements the only semantic subheading and locks the skipped hierarchy into the focused test. Use h2 here, or introduce a real h2 for the active sub-event before retaining h3.

Verified:

  • Exact stacked base codex/settings-athlete-form-context-fix and exact three-file scope.
  • SubEventForm is reachable for parent events with childEvents; replacing FormLabel does not remove a field association because Movements is a collection backed by selectedMovements.
  • Selected/unselected aria-pressed rendering, empty state, and unchanged update payload/controller invalidation are covered and pass (3/3 focused tests).
  • WODsmith Start type-check, UI inventory freshness, page-coverage freshness, lat check, and all GitHub CI checks pass.
  • No unresolved review threads. CodeRabbit was triggered but its substantive review was rate-limited.

Verdict: request the heading-rank correction; otherwise the narrow fix and invariants look sound.

@zacjones93

Copy link
Copy Markdown
Contributor Author

Follow-up independent review receipt for exact head aeebcb7

The prior P2 accessibility finding is resolved. Movements is now an h2 directly beneath the route h1 in the actual heading tree, and the focused assertion correctly requires heading level 2. The follow-up changes only that element and its test expectation; diff check is clean.

Independent rerun: focused route tests pass 3/3 and lat check passes. No new findings. GitHub CI for this new head is still running at the time of this receipt; CodeRabbit reports success status but skipped the draft review.

Final verdict: LGTM, conditional only on the current CI rerun completing green.

@zacjones93
zacjones93 force-pushed the codex/series-event-movements-form-context-fix branch from aeebcb7 to aa3bc60 Compare July 10, 2026 22:59
@zacjones93

Copy link
Copy Markdown
Contributor Author

Post-rebase independent receipt for exact head aa3bc60 (current base be6c57a).

Verified the PR delta remains exactly three files: the series event route, its focused test, and series-event LAT documentation. No athlete browser-evidence, page-coverage evidence, or other PR627 artifacts leak into this diff. The prior P2 remains resolved: Movements is h2 and the focused assertion requires level 2. No new findings.

Current CI: Start/Crew lint, branch-stack, GitGuardian, and CodeRabbit status are green; builds and E2E shards are still running. Verdict: LGTM, pending the rebased-head CI run completing green.

@zacjones93
zacjones93 merged commit d6316c1 into codex/settings-athlete-form-context-fix Jul 15, 2026
53 of 184 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