Extend the Markdown formatting bar to list-item rows - #886
Merged
Conversation
List items render inline Markdown, but only text notes had a way to write it from a button. Both clients now put a formatting bar over list rows too, carrying bold, italic and strikethrough. The three block actions are deliberately absent. An item is lexed as inline content, so `## `, `- ` and `- [ ] ` stay literal source there (docs/specs/markdown-rendering.md §2.1) — a heading button would write characters guaranteed never to render, and a checkbox button would write a second checkbox next to the row's real one. Both clients drop the same three, so the bar still reads as one feature across them. Titles keep no bar at all; they are plain text everywhere. Where the bar lives differs per client, but not why: it must never take focus, because a row shows its source only while it holds the caret. - Webapp: one bar docked under the active items, aimed at the row with the caret. It hides rather than unmounts when no row is focused — the modal is centred, so a bar that came and went would grow the panel and shift every row under the pointer. visibility:hidden also takes the buttons out of the focus order, so an inert bar is inert to everyone. - Mobile: iOS attaches every row to an InputAccessoryView, so the bar docks above the keyboard for whichever row is focused. Android renders it inline above the action bar while a row is editing, with the clear deferred so tapping between rows does not flash it away and back. An item already at ITEM_TEXT_MAX_LENGTH drops the press with a toast rather than truncating: the markers are characters the user did not type, so truncating would eat the tail of their text. The webapp writes back through applyTextareaEdit, so a row edit stays undoable. Ctrl+B / Ctrl+I on list rows is tracked separately in #883. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016z9zXQGiJvDp6eLbVqzAPy
Contributor
WalkthroughAdds inline Markdown formatting toolbars for focused list items on web and mobile. Item toolbars expose bold, italic, and strikethrough controls. Web rows use stable textarea IDs and a shared toolbar outside scrollable content. Mobile rows expose selection handles and use iOS accessory or Android inline toolbars. Formatting restores selection, preserves focus and undo behavior, and rejects edits that exceed item length limits. Tests and localization strings cover the new behavior. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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 |
Placed in the modal's scrollable body, the bar sat wherever the list ended. Measured on a 25-item list with the caret in the top row: the scroll viewport ended at y=720 and the bar was at y=1209 — present, correct, and ~490px out of reach. It now sits between the scrolling body and the action bar, so it is always where the buttons are expected to be regardless of list length. Mobile's Android bar is pinned above the action bar for exactly this reason; the webapp now follows the same rule. The text-note bar keeps its place in flow under the textarea: a text note has one editing surface and the bar is attached to it, which a list of N rows has no equivalent of. Side effect worth naming: the always-rendered labels/avatars row is empty on a saved note with no labels, and the bar's top border used to frame that empty row. It now reads as ordinary trailing padding inside the scroll area. Adds an e2e regression test asserting the bar is in the viewport while the top row of a long list holds the caret. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016z9zXQGiJvDp6eLbVqzAPy
hanzei
marked this pull request as ready for review
August 10, 2026 20:57
hanzei
enabled auto-merge (squash)
August 10, 2026 21:08
hanzei
disabled auto-merge
August 10, 2026 21:08
Owner
Author
|
@coderabbitai review |
Contributor
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webapp/src/components/SortableItem.tsx (1)
338-345: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the item active while keyboard focus is in its formatting toolbar.
When a user presses Tab from a list-item textarea, line 345 schedules removal of
editingItemId. The toolbar is a valid tab stop, so it becomes invisible before the user can activate a button. The row also leaves source mode. Keyboard users cannot use the new item formatting controls.
webapp/src/components/SortableItem.tsx#L338-L345: preserve the row source mode while focus moves into its toolbar.webapp/src/components/MarkdownToolbar.tsx#L140-L153: report focus entering and leaving the toolbar.webapp/src/components/NoteModal.tsx#L1175-L1188: retain the active item while focus is within its toolbar, then clear it after focus leaves both the row and toolbar. Add a keyboard Tab coverage case.🤖 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 `@webapp/src/components/SortableItem.tsx` around lines 338 - 345, Keep the item in source/editing mode when focus moves from its textarea into the formatting toolbar: update SortableItem.tsx lines 338-345 to recognize toolbar focus, add toolbar focus-enter/leave callbacks in MarkdownToolbar.tsx lines 140-153, and update NoteModal.tsx lines 1175-1188 to retain editingItemId while focus is inside either the row or toolbar and clear it only after focus leaves both. Add keyboard Tab coverage for this behavior.
🤖 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 `@docs/specs/markdown-rendering.md`:
- Around line 601-607: Update the paragraph around the bar-form explanation to
remove trailing spaces from all inline code spans, especially the `## `, `- `,
and `- [ ] ` examples, while preserving their meaning as literal source syntax
and satisfying markdownlint MD038.
---
Outside diff comments:
In `@webapp/src/components/SortableItem.tsx`:
- Around line 338-345: Keep the item in source/editing mode when focus moves
from its textarea into the formatting toolbar: update SortableItem.tsx lines
338-345 to recognize toolbar focus, add toolbar focus-enter/leave callbacks in
MarkdownToolbar.tsx lines 140-153, and update NoteModal.tsx lines 1175-1188 to
retain editingItemId while focus is inside either the row or toolbar and clear
it only after focus leaves both. Add keyboard Tab coverage for this behavior.
🪄 Autofix
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 Plus
Run ID: 0e5290cc-59aa-4241-8511-1b9e5d80ad53
📒 Files selected for processing (21)
docs/specs/markdown-rendering.mdmobile/__tests__/NoteEditorScreen.item-formatting-bar.test.tsxmobile/src/components/ListItem.tsxmobile/src/i18n/locales/de.jsonmobile/src/i18n/locales/en.jsonmobile/src/i18n/locales/es.jsonmobile/src/i18n/locales/fr.jsonmobile/src/i18n/locales/it.jsonmobile/src/i18n/locales/nl.jsonmobile/src/i18n/locales/pl.jsonmobile/src/i18n/locales/pt.jsonmobile/src/screens/NoteEditorScreen.tsxmobile/src/screens/noteEditor/CheckedItemsSection.tsxmobile/src/screens/noteEditor/EditorToolbars.tsxwebapp/e2e/pages/DashboardPage.tswebapp/e2e/tests/markdown.spec.tswebapp/src/components/MarkdownToolbar.tsxwebapp/src/components/NoteModal.tsxwebapp/src/components/SortableItem.tsxwebapp/src/components/__tests__/NoteModal.test.tsxwebapp/src/utils/noteItems.ts
#884 landed the same feature's keyboard half while this branch was open, and the two collided on purpose-built code: - Both added an `applyItemMarkdownEdit`. Kept #884's — its signature takes the row's textarea from the keydown event, and its deferred caret restore is simpler than the pending-ref-plus-effect here. The toolbar resolves the row through itemInputRefs and calls the same function, so button and shortcut cannot diverge. - #884 dropped a press at the item cap silently, matching handleListContinuation. This keeps the toast instead: the function now backs a visible button as well as a shortcut, and mobile's bar already says why nothing happened. Deliberate change to that path. - Spec §5.1 and the e2e list-row block keep both sides' additions; the "unlike the six-button bar" contrast in #884's spec text no longer held, since the bar now reaches list rows too. Fixes a keyboard defect found while adding Tab coverage for the merged result: Tab out of a row's field lands on that row's own delete control before the toolbar, so clearing the editing row on the bare blur hid the bar before Tab could reach it — and the next Tab went past it to the action bar. The bar was unreachable by keyboard entirely. The clear is now deferred and asks where focus actually settled, keeping the bar for anything inside a row or the toolbar, and keeping the row on its source form so the selection the next press acts on stays visible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016z9zXQGiJvDp6eLbVqzAPy
hanzei
enabled auto-merge (squash)
August 11, 2026 05:11
hanzei
disabled auto-merge
August 11, 2026 05:11
hanzei
added a commit
that referenced
this pull request
Aug 11, 2026
The list bar was moved out of the modal's scrolling body and pinned above the action bar in #886; the content bar was left in flow under the textarea, on the reasoning that a text note has one editing surface the bar can attach to. That reasoning does not survive a long note. The textarea grows to fit its content, so the bar goes with it: on a 40-line note it sits below the viewport while the caret is at the top — the same failure the list bar had at 25 items, reached by a different route. Mobile already pins both of its bars above the action bar for exactly this reason. Both variants now render from one slot between the scrolling body and the action bar, swapping only the button set (`variant`), the action handler and — for list rows — the focus-settled blur callback. A text note's edit mode is not focus-driven (it ends on Escape, Done or a click outside), so the content bar has nothing to report back and passes no `onBlurOut`. Consequences worth naming: - The slot is reserved for text notes too, so a note collapsed to its preview shows ~34px of empty space above the action bar rather than the panel resizing every time editing starts. This is what a list note with no row focused already does. - Tab out of the textarea now passes the body's remaining controls (the labels button, share avatars) before reaching the bar, since DOM order follows the visual order. It costs nothing because leaving the field does not end the edit; the keyboard-focus spec asserts the new order. - One toolbar instance now serves both variants, so the new-note type selector can shorten the button set under a live roving tabindex. The index is clamped to the buttons that exist — left dangling it would give no button `tabIndex=0` and drop the bar out of the tab order. Adds an e2e regression test asserting the bar is in the viewport with the caret at the top of a 60-line note, the text-note counterpart of the long-list one, plus unit coverage for the reserved slot and the clamp. Claude-Session: https://claude.ai/code/session_01JMybPS1iBFR2XWkMRjtQjT Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
List items render inline Markdown, but only text notes had a way to write it from a button. Both clients now put a formatting bar over list rows too — bold, italic, strikethrough.
Why three buttons and not six
The block actions are deliberately absent. An item is lexed as inline content, so
## x,- xand- [ ] xstay literal source there (spec §2.1):##that renders as those characters ❌Both clients drop the same three, so the bar still reads as one feature across them. Note titles keep no bar at all — they are plain text everywhere (§1).
Where the bar lives
Both clients dock the item bar to the editor's chrome rather than laying it out with the rows. A list is as long as the user makes it, so a bar in the scrolling content sits wherever the list ends. Measured on a 25-item list with the caret in the top row, the first version of this PR put the bar at y=1209 while the scroll viewport ended at y=720 — present, correct, and ~490px out of reach. It now sits between the scrolling body and the action bar; mobile's Android bar was already pinned for the same reason.
visibility: hidden, keeping its slot) while none does. The reserved slot is load-bearing: the modal is centred, so a bar that mounted and unmounted would grow the panel and shift every row under the pointer.visibility: hiddenalso drops the buttons from the focus order and the a11y tree.nativeID, so anInputAccessoryViewdocks above the keyboard for whichever row is focused.The content bar keeps its old place in flow under the textarea: a text note has one editing surface and the bar is attached to it. Only a list has N rows for one bar to serve.
Keyboard reachability
Found while adding Tab coverage, and worth calling out because it was a real defect: Tab out of a row's field lands on that row's own delete control first, then its assignee control, and only then the toolbar. Clearing the editing row on the bare blur hid the bar before Tab could reach it — and the next Tab then went straight past it to the action bar. The toolbar was unreachable by keyboard entirely.
The clear is now deferred and asks where focus actually settled: anything inside a row or inside the toolbar keeps the bar, and the row keeps showing source so the selection the next press acts on stays visible. The same deferral is what stops row-to-row movement flickering the bar. Covered by three unit tests and an e2e test that Tabs the whole path and presses Enter on Bold; all four fail if the settle-time check is removed.
Relationship to #884
The two branches collided on purpose-built code, resolved as:
applyItemMarkdownEdit. Kept webapp: wire Ctrl/Cmd+B and +I to list-item rows #884's — its signature takes the row's textarea from the keydown event, and its deferred caret restore is simpler than the pending-ref-plus-effect this branch had. The toolbar resolves the row throughitemInputRefsand calls the same function, so button and shortcut cannot diverge.handleListContinuation). This keeps a toast instead — the function now backs a visible button as well as a shortcut, a button that silently does nothing reads as broken, and mobile's bar already says why. Happy to revert that if you'd rather keep the shortcut silent.Other behaviour worth calling out
note.itemLimitReached, new key, all 8 mobile locales).applyTextareaEdit, soexecCommandreplays it as a user edit rather than emptying the browser's undo stack.API-breaking changes
None. No server change, no schema change, no API surface touched.
Testing
task check— green.task test-e2e— 388 passed, 1 failed (checked-items-bulk-actions.spec.ts). That spec is flaky on master too: a baseline full run ondf405dfunder identical conditions gave 4 failures — two in that same file, plustoast-timing.spec.tsandnotes.spec.ts. Across four runs this branch produced 2, 0, 1 and 1 failures, all inside that pre-existing flaky set.NoteModal.test.tsx, a newmobile/__tests__/NoteEditorScreen.item-formatting-bar.test.tsx(8 cases), and 6 e2e cases inmarkdown.spec.ts— including the long-list viewport regression and the Tab path above.Docs
docs/specs/markdown-rendering.md§5.1 rewritten for the two variants, why both clients dock the bar rather than inline it, the settle-time focus rule, and the §7 testing map.Screenshots
Captured locally in both themes and on a long list. I have no way to attach binaries to a PR from this environment, so they are not inline here — happy to have them added if that blocks review.
Follow-up
Unrelated, noticed while measuring: the labels/avatars row at
NoteModal.tsxrenders unconditionally and collapses to zero height on a saved note with no labels or collaborators, costing 32px ofspace-y-4around nothing. Pre-existing; left alone to keep this diff scoped.