Uh oh!
There was an error while loading. Please reload this page.
fix(comments): rebuild the comment composer on the create-post UX - #6417
fix(comments): rebuild the comment composer on the create-post UX#6417tsahimatsliah wants to merge 24 commits into
Conversation
The reported bug: on mobile a long comment overflowed with nowhere to scroll, so the text ended up hidden behind the virtual keyboard. Root cause was two-fold. `CommentMarkdownInput` drove `RichTextInput` with the default top toolbar, which renders no scroll container and no max-height, so the editor just grew. `CommentModal` then force-sized the form to the visual viewport by writing `style.height` during render — producing the literal "autopx" whenever the maths went negative. The composer is now the same inline box as the watercooler create-post composer, minus title, cover, post type and audience: bottom toolbar, one action bar, and a height capped against `visualViewport` (the part that survives the keyboard) with the body scrolling inside it. Same component on every viewport, so `CommentModal` and the mobile/desktop fork are gone. The mobile floating bar now opens the in-page composer through a window event instead of mounting its own copy inside the fixed footer. Several fixes landed in shared primitives and so reach every composer, including create post: - RichTextInput: rich and markdown modes share one tree, so switching no longer remounts the avatar (which refetched the image and blinked) or the action bar. Heights now match between modes, and the scroll offset and caret are preserved across the toggle. - RichTextToolbar: the overflow budget ignored dividers and row gaps, and the overflow button lived inside the `overflow-hidden` group it exists to protect, so it got sliced. Also swapped to the three-dots icon and kept it beside the formatting items. - Drawer: closing on any click whose target was outside the panel also caught portaled dropdowns, whose synthetic clicks bubble up the React tree. Picking a post type, audience, poll duration or schedule tore the drawer down instead of running the action. Now only a backdrop hit closes it. - Switch: the label could not shrink, so long copy ran off the right edge on mobile. - ProseMirror no longer adds a second min-height inside the editor's own padding. - AudienceChip and the poll option input could not shrink either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Swapping the rich editor for the textarea briefly shrinks the scroll container, so the browser clamps its offset to 0. Restoring from a later animation frame was too late in one direction: markdown to rich held, but rich to markdown always landed back at the top. Restore right after the textarea gets its real height, before paint, guarded by a flag so normal typing does not fight the user's own scrolling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Engineering review pass over the composer. No behaviour or visual change. - Stories move under stories/components/comments/, next to the existing CommentContainer story, and are retitled Components/Comments/*. The storybook AGENTS.md groups stories by atomic design, and a top-level `comments/` group also sorted outside preview.tsx's storySort order. Added the `autodocs` tag the same guide asks for. - The header label goes through Typography rather than a raw span with typo-* classes, per the components guide. - The composer form carries an aria-label, so it exposes role="form" to assistive tech. The spec queries that instead of a data-testid, which the components guide reserves for when no accessible query exists. - Drop three props left dead by removing CommentModal: `style`, `showUserAvatar` and `CommentClassName.markdownContainer`. Nothing passed them any more, and the comment composer always wants its avatar. - Flatten CommentInput's `className.input` nesting to `CommentClassName`. The wrapper object only existed to sit beside the `modal` key that went with the deleted modal branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The changed-file guard reports all strict errors in a touched file, not just newly introduced ones, so editing these files made their existing violations block CI. Fixed rather than suppressed; no behaviour change. - isSourcePublicSquad accepts an optional Source, which is what its body already assumes via optional chaining. - PostEngagements: useRef<NewCommentRef>(null) for a LegacyRef-compatible ref, and `false | value` unions replaced with undefined branches. - Drawer: container ref nullable, forwardRef render fn typed with ForwardedRef. The debounced close now forwards the event that triggered it instead of dropping it and calling onClose with undefined. - PostComments.modalParentSelector may return null, matching getCompanionWrapper; the call site already null-coalesced it. - CompanionDiscussion returns ReactElement | null, which it always did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mobile round of feedback on the composer surfaces: - On mobile the comment composer opens as a full-screen Drawer, the same way creating a post does — an inline box wedged into the page is too small to write in. It autofocuses so the keyboard opens ready to type. A new `fills` mode makes the composer fill the drawer instead of capping against the viewport, dropping the inline card treatment. - Full-screen drawers size themselves to the visual viewport (height and offsetTop), so bottom action bars sit above the virtual keyboard instead of underneath it. useVisualViewport now reports offsetTop and listens to viewport scroll for the iOS keyboard-pan case. - The create-post kind picker moves to its own row above the action bar on mobile (`stackToolbarLeading`). The bar's overflow menu only relocates formatting items, so the picker plus the icon actions overflowed the clipped group and rendered sliced. - Expand/collapse is hidden on mobile — the composer is already full-screen there, so the control was a no-op. - Drawer wrapper overrides use `!px-0 !pt-0` (matching NavDrawer): plain `p-0` lost to the drawer's default padding, so the composer paid padding twice, and killing padding-bottom outright would also have dropped the safe-area inset that `drawer-padding` provides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Animated/transformed ancestors (the animate-composer-in wrapper) create a containing block for position: fixed, pinning the drawer mid-page with the page showing through. appendOnRoot portals it to the document root so it actually covers the screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er on mobile - The stacked leading row rendered in addition to the inline slot rather than instead of it, so mobile showed two kind pickers. The inline slot now yields when stackLeading is set. - On mobile the schedule calendar moves up beside the header's scheduled- posts clock, where scheduling already lives; the bottom bar keeps just the Post button. Desktop keeps both beside Post. Specs for this mobile round: toolbar leading-slot dedupe, the comment drawer (full-screen, portaled, autofocus, fills, padding override), keyboard-safe drawer sizing, backdrop-only close, header expand/schedule placement per breakpoint, and the composer fills mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…loating bar on redesign - Queue an early imperative focus until the TipTap editor exists, so reply composers autofocus on desktop (the mount-time focus raced editor creation) - Move the More-formatting tooltip onto the trigger's tooltip prop; the Tooltip wrapper blurred the trigger on mouseup, insta-dismissing the menu - Extract useOpenPostCommentRequest and register it in PostDiscussionPanel too, so the mobile floating bar opens the composer on the redesigned post page, not only the classic one - Cover the full PR surface with tests: useVisualViewport, postComment event bridge, Switch wrap fix, RichTextInput avatar/mode/focus, FooterWrapper dispatch, PostPage integration, toolbar overflow menu Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scrolling inside the mobile composer drawer chained to the post page behind it, which visibly jumped and shifted. Apply the same body lock react-modal uses (hidden-scrollbar, ref-counted for stacked drawers) and contain overscroll on the drawer's own scroller; the composer's internal scrolling is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
<html> is the page's actual scroll container, so the body-level lock never reached the viewport and the page behind the drawer kept scrolling. Verified against the live preview. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The drawer's safe-area bottom padding stacked with the action bar's own pb-5, reading as double spacing under the actions. The bar now absorbs the safe area itself (max(1.25rem, safe-area-inset-bottom)) and the drawer adds none, so the composer sits 20px from every edge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The header and avatar hugged the comment list's 16px guideline while the action bar carried 20px, so the drawer's edges did not match. In the drawer there is no list to line up with, so both join the 20px frame the action bar and create-post composer already use; inline keeps 16px. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tsahimatsliah
left a comment
There was a problem hiding this comment.
Re-review (previous comments cleared at your request)
Big jump since the last pass, and most of it is in the right direction. What I checked and now consider resolved:
- The event contract is no longer a one-off.
useOpenPostCommentRequestgives it a name, a doc comment that states the obligation, and a spec — andPostDiscussionPanelnow registers too, so the focus layout can't silently swallow the tap.FooterWrapperhas a test asserting it asks rather than mounting its own composer. This was my main worry last time; it's properly closed. useVisualViewporthandles the iOS case. TrackingoffsetTopand subscribing toscrollis the correct fix for iOS panning the layout viewport under the keyboard rather than resizing it, and the three spec cases pin exactly that.Drawernow has real coverage — backdrop-only close, viewport sizing, scroll lock, and the stacking case.- Specs for the toolbar's leading/overflow behaviour,
Switch,SmartComposerModalandpostCommentfill in most of what was untested.
CI is green. Four things below.
Blocking
1. The description no longer describes the PR. "Still outstanding (not in this PR)" lists three items — full-screen mobile drawer, tighter mobile padding, pinning the action bar above the keyboard — and all three have since shipped here (CommentInput's Drawer isFullScreen, the 20px frame commits, and the visual-viewport drawer sizing). The stated reason for deferring them was that they "need device verification", and the iOS/Android boxes are still unticked. Please update the body, because a reviewer reading it will skip precisely the part that changed most.
2. Still no device pass, and the surface is now more mobile-specific than it was. Last pass this was about a keyboard fix; it is now a full-screen drawer, a page scroll lock, and an overlay positioned from visualViewport.offsetTop. Those are the three things that behave differently on real iOS Safari than in any emulator — offsetTop in particular. One pass on an actual phone (iOS Safari + Chrome, Android) before merge.
Non-blocking
3. ProseMirror { min-h-[6rem] → h-full } is unchanged from the last pass, and still global.height: 100% resolves to auto under an ancestor with only min-height. Inside the composer and TextForm the editor is a stretched flex item so it resolves — but ShareLink, WriteFreeformContent and LiveRoomChatPanel take their box height from minHeightClassName on EditorContent, and there the editable element can collapse to its content. The box still looks right; clicking the empty space below the first line may stop focusing. Still not in the verification list — a click into each would settle it.
4. The scroll lock is global to every Drawer, and clobbers rather than restores. Every drawer in the app now hides the page scrollbar and sets overflow: hidden on <html> — including mobile context menus and pickers that never locked before, and any desktop drawer, where removing the scrollbar can shift layout. The teardown does removeProperty('overflow'), which deletes whatever was there rather than restoring a previous inline value, so it will fight any other lock (react-modal, a future one) instead of nesting with it. Capturing the previous value and restoring it would make the reference counting honest.
5. useVisualViewport() runs in every BaseDrawer, not just full-screen ones. The result is only used behind isFullScreen, but the hook is called unconditionally, and it now subscribes to scroll as well as resize — which on iOS fires continuously while the keyboard is open. That re-renders every mounted drawer subtree, ProseMirror included. Gating the subscription on isFullScreen (or rAF-throttling the setState) would keep the fix without the cost.
6. The full-screen fork is at Laptop, not Tablet.!useViewSize(ViewSize.Laptop) means everything below 1020px, so an iPad now gets a full-screen sheet for a comment where the old fork (!Tablet) gave it the inline box. It matches SmartComposerModal, so it may well be deliberate — worth confirming, since the description frames this as a phone change.
7. Small leftover:rows: 7 is still passed in textareaProps, but the auto-grow deliberately measures from 0px so rows can't act as a floor, and minHeightClassName="min-h-[6rem]" sets the empty height. It reads as load-bearing and isn't.
Verification
- Re-read the full three-dot diff against
main, and the new commits in isolation - Re-traced the event path, now through
useOpenPostCommentRequest, and confirmed both registrations - Read the new
Drawer,useVisualViewport,RichTextToolbar,FooterWrapperand composer specs - Checked the scroll-lock teardown and the
visualViewportsubscription against everyDrawerconsumer, not just the composer - Confirmed the earlier
TextForm/StandupFormsubmit-duplication andDrawerbackdrop findings remain correctly handled - CI green
- No device verification — items 1 and 2
- The three top-toolbar editors still not exercised — item 3
Verdict
Comment. The structural concerns from the last pass are addressed and the new tests are good. What's left is documentation and device confirmation rather than code — update the body, get one phone pass, and I'd be happy to recommend approval after that.
Reviewed by AI.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Drawer: lock the page only for full-screen drawers, so context menus and pickers keep the page scrollable exactly as before; capture and restore the inline html overflow instead of deleting whatever another lock set - useVisualViewport: take an `enabled` flag and subscribe only when asked; non-full-screen drawers no longer re-render on every iOS keyboard scroll - RichTextInput: fill the editor by flexing rather than `height: 100%`, which resolved to auto where the box height comes from `minHeightClassName` and let the editable collapse away from clicks below the first line - CommentMarkdownInput: drop the inert `rows: 7` Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MyFeedPage › should request user feed timed out in test_webapp. Known flaky async feed race — passes locally on repeat and this branch does not touch the feed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comments explaining what the code already says, narrating fixed bugs, or restating test names are removed; the ones that survive state constraints the code cannot show (platform quirks, cross-file contracts, do-not-readd traps). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tsahimatsliah
left a comment
There was a problem hiding this comment.
Final review
Third pass, against 265ebb3. Everything I raised last time is closed, and closed properly rather than papered over:
- Scroll lock is now scoped and honest — gated on
isFullScreen, previous inlineoverflowcaptured and restored, ref-counted for stacking, with tests for each branch. Context menus and pickers behave exactly as they did before. useVisualViewport(enabled)— partial drawers no longer subscribe toresize/scroll, so an open iOS keyboard doesn't re-render every mounted drawer subtree..ProseMirrorflexes instead ofheight: 100%— the right fix, and you verified theShareLinkcase on the preview rather than reasoning about it.min-h-fullwould have failed the same way, so the choice matters.- Description now matches the branch, and the outstanding-device-pass caveat is stated up front instead of listing shipped work as deferred.
- The
Laptopfork is answered: matchingSmartComposerModalso "writing a comment behaves like writing a post" holds on every non-laptop width.
I re-traced the event bridge over the whole surface this time, not just the two registrations: PostEngagements (classic) and PostDiscussionPanel (redesign, via PostFocusCard, rendered unconditionally) are the only surfaces FooterWrapper can sit under with a post, and EngagementRail drives its own ReaderFloatingActionBar rather than the footer bar — so there is no surface where the floating bar's tap lands on nothing. shouldHandleCommentQuery survived the FooterWrapper change because the page-level NewComment was only CSS-hidden on mobile, never unmounted. No dangling CommentInputOrModal / CommentModal references. CI green including typecheck_strict_changed.
Blocking
Only the one you already flagged: the real-device pass. Nothing in the code is blocking. But this branch now ships a full-screen drawer positioned from visualViewport.offsetTop, an <html>-level scroll lock, and env(safe-area-inset-bottom) padding — the three things emulation cannot honestly verify, and offsetTop in particular diverges on real iOS Safari. One pass on hardware (iOS Safari + Chrome, Android) and I'm happy to recommend merge.
Non-blocking
See the four inline notes. Two more that don't attach cleanly to a line:
Escape and focus trap left with CommentModal. react-modal gave the old composer Escape-to-close and a focus trap; Drawer has neither (no drawer in the repo does). Below Laptop includes an iPad with a keyboard, where the X is now the only way out. Consistent with existing drawer behaviour, so not a regression I'd hold the PR for — worth a follow-up if the composer becomes the main writing surface.
Two create-post deltas that read as intentional but aren't in the description.SmartComposerModal now renders notificationToggleNode in markdown mode (previously !isMarkdownMode), and the shared avatar block means CreateSharedPostModal shows its avatar in markdown mode too, at ml-4 mt-4 instead of ml-3 mt-3 with the editor's indent moving from ml-3 tablet:ml-0 to !pl-3. Both follow from unifying the two trees; a glance at that modal on mobile would confirm the spacing still reads right.
Verification
- Full three-dot diff re-read at
265ebb3, plus2dfc541..265ebb3in isolation - Every previous finding re-checked against the current code
- Event bridge traced across all four composer surfaces and both post-page variants
- Scroll-lock capture/restore and stacking semantics re-derived
- Searched for dangling references to the deleted modal and its props
- CI green,
typecheck_strict_changedincluded - Real-device pass — blocking item above
Verdict
Comment. The code is in good shape and I'd approve it on the strength of the review alone; what's left is hardware confirmation, which I can't do for you.
Reviewed by AI.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- CommentMarkdownInput passes enabled=!fills to useVisualViewport, so the mobile drawer stops re-rendering on iOS keyboard scroll for a value it discards - the toolbar overflow budget measures the rendered divider and the row's column-gap instead of hand-synced constants (jsdom keeps fallbacks) - the open comment composer owns its focus through the queued autofocus: the by-id helper races the lazy chunk and TipTap's async editor - a title-only freeform edit no longer demands a body to save Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changes
Reported by a user: "the post comment input is broken on mobile… the input element or its wrapper is not scrollable and my long comments overflow. When this happens I can no longer see my text, because it's hidden behind the virtual keyboard."
Two causes.
CommentMarkdownInputdroveRichTextInputwith the default top toolbar, which renders no scroll container and no max-height — so the editor grew without bound.CommentModalthen force-sized the form to the visual viewport by writingstyle.heightduring render, producing the literal value"autopx"whenever the maths went negative.Rather than patch the modal, the composer was rebuilt on the create-post UX (the watercooler inline composer), minus title, cover, post type and audience.
Comment composer
visualViewport(the part that survives the keyboard), body scrolling inside it.CommentModalis deleted;CommentInputOrModal→CommentInput.Laptop: a full-screenDrawer, matchingSmartComposerModal's own!isLaptopfork — the composer gets the whole screen and opens focused, so the keyboard comes up ready to type. Portalled withappendOnRoot, because animated ancestors (animate-composer-in) trapposition: fixedand pinned it mid-page.visualViewportheight andoffsetTop, so the action bar sits above the keyboard on iOS, which pans the layout viewport instead of resizing it.hidden-scrollbar+ inlineoverflowon<html>, ref-counted, previous value restored), and the drawer's own scroller usesoverscroll-contain. Scrolling the composer no longer drags the post page behind it.Replying to @user(comment author → post author → source), orEditing your comment. Markdown toggle + close on the right.useOpenPostCommentRequest) instead of mounting its own copy inside the fixed footer. BothPostEngagementsandPostDiscussionPanelregister, so the redesigned post page answers it too.hidden tablet:flex). Deliberate: it is the redesign's primary entry point under the post, and the floating bar's comment tap opens the same composer.Create post — same shared components, so these land there as well
Shared primitives
focus()requested before the editor exists is queued rather than dropped, which is what left reply composers unfocused.overflow-hiddengroup it exists to protect, so it was sliced. Now three-dots, beside the formatting items. Its tooltip rides onDropdownMenuTrigger's owntooltipprop; wrapping the trigger inTooltipblurred it on mouseup and could dismiss the menu before it painted.offsetTopandscrollfor the iOS keyboard case, and takes anenabledflag so only consumers that need it subscribe.height: 100%, which resolved toautowherever the box height comes fromminHeightClassNameand let the editable collapse away from clicks below the first line.Tests / stories
New specs:
CommentMarkdownInput(16),CommentInput,Drawer(viewport sizing, backdrop-only close, scroll lock, stacking, overflow restore),useVisualViewport,useOpenPostCommentRequest,RichTextToolbar,Switch,postComment,FooterWrapper, plus additions toRichTextInput,SmartComposerModalandPostPage.New stories under
stories/components/comments/—Components/Comments/Composer(13 cases) andComponents/Comments/Composer states(all-states gallery + mobile widths).Conventions / types
Typography; the composer form carries anaria-label, so it exposesrole="form"and the spec queries that instead of adata-testid.CommentModal:style,showUserAvatar,CommentClassName.markdownContainer, and theclassName.inputnesting that only existed beside the oldmodalkey.PostEngagements,Drawer,CompanionDiscussion,PostComments,isSourcePublicSquad) — the changed-file guard reports all errors in a touched file, not just new ones. No behaviour change, exceptDrawernow forwards the triggering event toonCloseinstead of passingundefined.Events
No new tracking events.
Experiment
No new experiments.
Manual Testing
Measured on the preview rather than eyeballed, at 375px and 1280px:
0 → 812, editor focused on open, submit 32px above the bottom, one 20px frame on all four sides.End/PageDownmove the page 0px; the same keys after closing scroll it 150 → 550. Composer content still scrolls internally (562px of travel with 40 lines) and the submit stays on screen.On those affected packages:
CompanionDiscussionupdated, tests pass, not exercised by handDid you test the modified components media queries?
Did you test on actual mobile devices?
Important
Still needs a real-device pass before merge. The keyboard path is verified by measurement in emulation only, and
visualViewport.offsetTop— which positions the drawer while the keyboard is open — is exactly the API that behaves differently on real iOS Safari. Theenv(safe-area-inset-bottom)branch of the action bar's padding also only activates on hardware.🤖 Generated with Claude Code
Preview domain
https://claude-mobile-comment-input-fix.preview.app.daily.dev