Skip to content

Dock the text note's formatting bar like the list one - #892

Merged
hanzei merged 1 commit into
masterfrom
claude/text-notes-toolbar-style-s41i12
Aug 11, 2026
Merged

Dock the text note's formatting bar like the list one#892
hanzei merged 1 commit into
masterfrom
claude/text-notes-toolbar-style-s41i12

Conversation

@hanzei

@hanzei hanzei commented Aug 11, 2026

Copy link
Copy Markdown
Owner

#886 moved the list bar out of the modal's scrolling body and pinned it above the action bar. The content bar was left in flow under the textarea, on the reasoning that a text note has one editing surface for the bar to 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, so this also puts the two clients back in step.

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 the panel), so the content bar has nothing to report back and passes no onBlurOut.

Screenshots

Captured but not attachable from the environment this was written in — they are with the author to drop in. What they show:

  • A long text note, before and after. Before: the modal ends in bare text with no bar anywhere on screen. After: the bar sits above the action bar with the caret still on line 1.
  • A short text note being edited, before and after. The bar moves from inset-above-the-labels-row to flush against the action bar — which is where a list note has always put it.
  • Text and list side by side, after. Same slot, same styling, only the button set differs (six buttons vs. the inline three).
  • A text note collapsed to its preview. The reserved slot, reading as trailing padding above the action bar.

Consequences worth naming

  • The slot is reserved for text notes too. A note collapsed to its preview shows ~34px of empty space above the action bar rather than the panel resizing every time editing starts — the same trade a list note with no row focused already makes, and for the same reason (the modal is centred, so a bar that mounts and unmounts shifts the content under the pointer).
  • Tab order changed for text notes. 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. keyboard-focus.spec.ts asserts the new order; the toolbar is still one tab stop with arrow-key navigation.
  • 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 entirely.

Tests

  • New e2e: the bar is in the viewport with the caret at the top of a 60-line note (the text-note counterpart of the existing long-list test), and it keeps its slot while the note is collapsed to its preview.
  • New unit: the reserved slot and aria-controls across the edit/preview swap, and the roving-tabindex clamp when the type selector swaps the button set.
  • Updated keyboard-focus.spec.ts for the tab order above.
  • task check and task test-e2e both pass (393 e2e tests).

docs/specs/markdown-rendering.md §5.1 is updated: the placement rule now covers both bars, and the paragraph saying the content bar keeps its in-flow place is gone.

No API changes; webapp only.

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.

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

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6df1fab1-b5af-4380-9729-c10b7e2e613c

📥 Commits

Reviewing files that changed from the base of the PR and between 719c7bc and 9a68f24.

📒 Files selected for processing (6)
  • docs/specs/markdown-rendering.md
  • webapp/e2e/tests/keyboard-focus.spec.ts
  • webapp/e2e/tests/markdown.spec.ts
  • webapp/src/components/MarkdownToolbar.tsx
  • webapp/src/components/NoteModal.tsx
  • webapp/src/components/__tests__/NoteModal.test.tsx

Walkthrough

The modal now renders one docked Markdown toolbar for text and list-item editing. The toolbar targets the active textarea, remains hidden while preserving its slot when inactive, and exposes aria-controls only when a target exists. Toolbar focus remains a single roving tab stop, with the active index clamped when actions change. Tests cover preview transitions, read-only notes, keyboard focus, accessibility linkage, and viewport visibility.

Possibly related PRs

  • hanzei/jot#841: Introduced the Markdown toolbar that this change refines.
  • hanzei/jot#868: Added list-item rendered/source editing that this toolbar now supports.
  • hanzei/jot#884: Updated NoteModal list-item Markdown editing behavior.

Poem

A rabbit finds the toolbar docked just right,
One shared bar for every note in sight.
Tabs hop once, then arrows steer,
Hidden slots stay calm and clear.
“Markdown magic!” the rabbit cheers.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: docking the text note formatting bar to match the list note behavior.
Description check ✅ Passed The description directly explains the toolbar docking, shared slot, behavior changes, tests, and documentation updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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.

@hanzei
hanzei marked this pull request as ready for review August 11, 2026 08:26
@hanzei
hanzei merged commit 858acd2 into master Aug 11, 2026
12 checks passed
@hanzei
hanzei deleted the claude/text-notes-toolbar-style-s41i12 branch August 11, 2026 08:34
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