Split NoteComposer into focused state and UI modules#358
Conversation
Move draft persistence, media handling, poll state, submission, and Relay updates into focused modules while preserving the composer's public API and existing behavior. Add focused state tests for draft synchronization, uploads, polls, Relay updates, and submission input. Closes hackers-pub#341 Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughNoteComposer was split into focused draft, media, poll, and submission controllers, with extracted media and poll editors, Relay update helpers, state utilities, and dedicated tests. ChangesNote composer refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant NoteComposer
participant Controllers
participant Relay
participant LocalStorage
NoteComposer->>Controllers: route media, poll, draft, and submit actions
Controllers->>LocalStorage: restore and persist drafts
Controllers->>Relay: upload, generate alt text, and submit mutations
Relay-->>Controllers: return media and mutation results
Controllers-->>NoteComposer: update reactive UI state
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the NoteComposer component by modularizing its logic into dedicated controllers (createDraftController, createMediaController, createPollController, and createSubmissionController) and sub-components (MediaEditor and PollEditor). It also introduces comprehensive unit tests for the newly created state modules. The review feedback highlights several import sorting violations across the new files, specifically noting that relative imports should be grouped and sorted alphabetically before absolute alias imports (starting with ~/) to comply with the repository's style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d92f5c9545
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep relative composer-module imports ahead of aliased imports and sort the extracted controller imports consistently. hackers-pub#358 (comment) hackers-pub#358 (comment) hackers-pub#358 (comment) hackers-pub#358 (comment) hackers-pub#358 (comment) hackers-pub#358 (comment) Assisted-by: Codex:gpt-5.6-sol
Render media and poll rows with Solid's index-keyed helper so immutable state updates do not remount active text fields or reset their carets. hackers-pub#358 (comment) hackers-pub#358 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the NoteComposer component by modularizing its state and logic into dedicated controllers and sub-components (draft, media, poll, and submission controllers) and adding comprehensive unit tests. The review feedback suggests sorting the imports in NoteComposer.tsx alphabetically within their groups to comply with the repository style guide, and adding defensive null/undefined checks on mutation responses in createSubmissionController.ts to prevent potential runtime crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Keep external dependencies and internal helpers in alphabetical order so the extracted composer modules follow the repository import convention. hackers-pub#358 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the NoteComposer component by modularizing its logic into separate controllers and state files (such as createDraftController, createMediaController, createPollController, and createSubmissionController) and introducing comprehensive unit tests. The review feedback focuses on improving robustness during submission validation, specifically suggesting to explicitly verify that all media items have a valid uuid before posting and to avoid non-null assertions (item.uuid!) by using flatMap to filter out incomplete media items gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
web-next/src/components/note-composer/PollEditor.tsx (1)
15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDestructure the component props.
Prefer
PollEditor({ poll }: PollEditorProps)and referencepolldirectly.As per coding guidelines,
**/*.{tsx,jsx}components must use props destructuring.🤖 Prompt for 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. In `@web-next/src/components/note-composer/PollEditor.tsx` around lines 15 - 16, Update the PollEditor component signature to destructure its props, accepting poll directly in the parameter list instead of a props object. Replace any props.poll references within PollEditor with the destructured poll variable, while preserving the existing behavior.Source: Coding guidelines
🤖 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-next/src/components/note-composer/createSubmissionController.ts`:
- Around line 294-300: Update all four mutation onError callbacks in
createSubmissionController, including the handlers near the existing toast
blocks, to log each failure through LogTape before showing the toast. Use the
file’s established logger and structured contextual fields identifying the
failed operation, while preserving the current error toast behavior.
- Line 442: Update the flow around saveDraftNow() to inspect its boolean result
and stop before the article-draft mutation when it returns false; continue only
when the draft flush succeeds.
In `@web-next/src/components/note-composer/PollEditor.tsx`:
- Around line 58-73: Add complementary aria-pressed attributes to the two choice
Buttons in PollEditor, setting Single choice to the inverse of
props.poll.multiple() and Multiple choice to props.poll.multiple(). Keep the
existing visual variants and click handlers unchanged.
In `@web-next/src/components/note-composer/submissionState.ts`:
- Around line 81-87: Update the submission validation flow around the media
checks to reject any completed media item whose upload uuid is missing before
constructing the mutation input. Preserve the existing uploading and missing-alt
errors, add the appropriate failed-upload validation result, and add a
regression test covering { uploading: false, uuid: undefined } so no mutation
input contains an undefined mediumId.
---
Nitpick comments:
In `@web-next/src/components/note-composer/PollEditor.tsx`:
- Around line 15-16: Update the PollEditor component signature to destructure
its props, accepting poll directly in the parameter list instead of a props
object. Replace any props.poll references within PollEditor with the
destructured poll variable, while preserving the existing behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b8f6521-0b1e-44fa-acd2-b346bd566534
📒 Files selected for processing (17)
web-next/src/components/NoteComposer.tsxweb-next/src/components/note-composer/MediaEditor.tsxweb-next/src/components/note-composer/PollEditor.tsxweb-next/src/components/note-composer/createDraftController.tsweb-next/src/components/note-composer/createMediaController.tsweb-next/src/components/note-composer/createPollController.tsweb-next/src/components/note-composer/createSubmissionController.tsweb-next/src/components/note-composer/draftState.test.tsweb-next/src/components/note-composer/draftState.tsweb-next/src/components/note-composer/mediaState.test.tsweb-next/src/components/note-composer/mediaState.tsweb-next/src/components/note-composer/pollState.test.tsweb-next/src/components/note-composer/pollState.tsweb-next/src/components/note-composer/relayUpdates.test.tsweb-next/src/components/note-composer/relayUpdates.tsweb-next/src/components/note-composer/submissionState.test.tsweb-next/src/components/note-composer/submissionState.ts
👮 Files not reviewed due to content moderation or server errors (7)
- web-next/src/components/note-composer/createDraftController.ts
- web-next/src/components/note-composer/draftState.ts
- web-next/src/components/note-composer/createMediaController.ts
- web-next/src/components/note-composer/MediaEditor.tsx
- web-next/src/components/note-composer/pollState.ts
- web-next/src/components/note-composer/createPollController.ts
- web-next/src/components/NoteComposer.tsx
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Reject completed media without a valid upload UUID and carry the validated media type into mutation input construction. This removes the non-null assertion and prevents undefined medium IDs from reaching GraphQL. hackers-pub#358 (comment) hackers-pub#358 (comment) hackers-pub#358 (comment) Assisted-by: Codex:gpt-5.6-sol
Mark the single-choice and multiple-choice controls as pressed according to the active mode so assistive technology receives the visual state. hackers-pub#358 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #341.
Why
Recent composer fixes have had to account for draft synchronization, async media work, poll state, mutation completion, and Relay updates at the same time. Those concerns shared one component and one lifecycle, so changing a single path meant reasoning about every other path that could react to the same state.
Keeping web-next/src/components/NoteComposer.tsx small is secondary. The useful change is putting each side effect next to the state it governs, making cross-feature dependencies visible at the orchestration layer, and giving the fragile transitions direct test coverage.
How
web-next/src/components/NoteComposer.tsx remains the public entry point. Its props and caller-facing behavior stay unchanged, while it now coordinates a set of modules under web-next/src/components/note-composer/. This keeps the refactor local to the composer instead of forcing an unrelated call-site migration.
The extraction follows one rule. Pure functions describe serializable state and transitions, controllers own browser or Relay side effects, and the presentational components receive narrow controller interfaces. This lets the tests exercise decisions without mounting the entire composer, while the top-level component still owns the interactions between content, language, reply and quote previews, acting accounts, and article switching.
The draft controller keeps storage scope changes, cross-composer synchronization, and delayed writes together. In particular, it preserves a dirty form when a stale composer publishes the same draft scope, then writes the active state back without starting a notification loop. Media state uses the same separation for upload progress, preview URL cleanup, draft-media verification, and generated alt text cancellation, so late async results can only update media that still exists.
Poll validation and submission input building are pure modules. The submission controller owns the four mutation paths and delegates Relay bookkeeping to a small updater that checks for a successful payload before appending a reply or incrementing its count. The media and poll editors were split out only where a narrow interface was already available, which avoids moving shared composer state into a second UI layer.
The split was driven by focused tests around the state transitions most likely to regress: draft scope changes, transient media state, poll restoration and validation, submission normalization, and Relay updates for successful and error payloads.
Verification
mise run checkpasses.mise run testpasses.mise run build:web-nextpasses.