Skip to content

fix(editor): add mobile indent/outdent controls for lists (#51) - #62

Merged
imaustink merged 2 commits into
mainfrom
fix/mobile-list-indent-51
Sep 29, 2026
Merged

imaustink merged 2 commits into
mainfrom
fix/mobile-list-indent-51

Conversation

@k5s-bot

@k5s-bot k5s-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #51 — "Users are unable to indent lists in mobile view."

List nesting in the editor relies on Tab / Shift-Tab (StarterKit's ListItem keyboard shortcuts). Touch keyboards on iOS and Android have no Tab key, so on mobile there was no way to indent (nest) or outdent a list item at all.

What changed

  • src/lib/editor/listIndent.ts (new) — small, dependency-light helper module wrapping ProseMirror's sinkListItem / liftListItem, plus a selectionInListItem check. Kept out of the Svelte component so it's unit-testable.
  • Editor.svelte — a compact on-screen toolbar with outdent and indent buttons that:
    • is mobile-only (shown under the existing max-width: 768px breakpoint; desktop keeps using Tab / Shift-Tab);
    • appears only when the editor is focused and the cursor is inside a list item;
    • stays pinned just above the on-screen keyboard using the visualViewport API;
    • uses pointerdown preventDefault so tapping a button doesn't blur the editor before the command runs.
  • Icon.svelte — added indent / outdent icons (Feather-style, matching the existing icon set).

The issue suggested adding a button to the iOS/Android keyboard itself. Web apps can't inject custom keys into the system keyboard, so the equivalent web-native solution is an in-editor toolbar surfaced on mobile — implemented here.

Testing

  • New unit tests in src/lib/editor/listIndent.test.ts cover indent, outdent, the top-level lift-out case, and the in-list selection check.
  • Full suite: 903 tests across 67 files pass (pnpm test).
  • svelte-check: 0 errors, 0 warnings.

Note: the toolbar's visibility and keyboard-inset positioning are driven by focus/selection state and the visualViewport API, which aren't exercised in the jsdom unit tests; the indent/outdent behaviour itself is covered.

Notes for other things spotted (not done, out of scope)

  • None required for this fix.

Maintainers: apply the ai-review label to this PR to request an automated code review, or the ai-triage label to have the review feedback addressed and the branch brought back in sync with its base. (The automation can't apply either label to its own PR, so a human needs to add it.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01ABfY4ijSST2YQYFZJSeTBG

Touch keyboards on iOS and Android have no Tab key, so the desktop
Tab / Shift-Tab shortcuts that nest and un-nest list items were
unreachable on mobile — there was no way to indent a list at all.

Add a small on-screen toolbar with indent and outdent buttons that
appears (mobile viewports only) when the editor is focused with the
cursor in a list item. It rides just above the on-screen keyboard via
the visualViewport API. The indent/outdent logic lives in a small,
unit-tested helper module (listIndent.ts) wrapping ProseMirror's
sinkListItem / liftListItem.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ABfY4ijSST2YQYFZJSeTBG
@k5s-bot

k5s-bot Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Starting work on this now.

@k5s-bot

k5s-bot Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Watch live or take over the session here: https://claude.ai/code/session_01GVtQavpUmth8CnsSgGfdS9

@k5s-bot k5s-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: approve — no changes requested.

Solid, well-scoped fix for #51. The indent/outdent logic is pulled into a testable listIndent.ts helper and the toolbar is correctly mobile-gated. A few things I specifically verified that hold up:

  • LIST_ITEM = 'listItem' matches the real schema — TaskLinkExtension extends StarterKit's ListItem (src/lib/editor/schema.ts:46,49), and it covers both bullet and ordered lists since both use listItem.
  • The sinkListItem/liftListItem chains return a meaningful boolean (the first-item no-op returns false), and the new unit tests assert exactly that.
  • position: fixed for .list-toolbar is safe — no transform/filter ancestor that would reparent the fixed positioning context.
  • updateKeyboardInset degrades sensibly across iOS (innerHeight stays, visualViewport.height shrinks → inset = keyboard height) and Android's resizes-content mode (layout viewport shrinks → inset ≈ 0, toolbar already sits above the keyboard).
  • onpointerdown+preventDefault reliably suppresses the focus/blur steal for touch input (the only path where the toolbar is visible), so the command runs before the editor blurs.

Test coverage is appropriate: indent, outdent, top-level lift-out, and the in-list selection check are all exercised. The toolbar's visibility/positioning is inherently DOM/visualViewport-driven and not reachable in jsdom, which the PR honestly calls out.

All CI checks are passing as of this review (frontend unit tests, svelte-check, e2e-local, e2e-api). Nothing blocking — good to go.

@k5s-bot

k5s-bot Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Review posted successfully.

Summary

I reviewed PR #62 ("fix(editor): add mobile indent/outdent controls for lists"). I checked out the branch fix/mobile-list-indent-51, examined the full diff across all four files, cross-checked the schema, positioning context, and viewport handling, and confirmed CI status.

Verdict: clean approval — no changes requested. Posted as a review comment on the PR (my integration can't submit an APPROVE event on the app-authored PR, so it's a COMMENTED review; no inline change requests were warranted).

What I verified

  • Correctness of the helper — LIST_ITEM = 'listItem' matches the real schema (TaskLinkExtension extends StarterKit's ListItem, schema.ts:46,49) and covers both bullet and ordered lists. The sinkListItem/liftListItem chains return a meaningful boolean, asserted by the new tests.
  • CSS positioning — position: fixed for the toolbar is safe; no transform/filter ancestor reparents the fixed context.
  • Cross-platform keyboard inset — updateKeyboardInset degrades sensibly on both iOS and Android's resizes-content mode.
  • Blur prevention — onpointerdown + preventDefault reliably suppresses focus-steal for touch input, the only path where the toolbar is shown.
  • Tests — indent, outdent, top-level lift-out, and the selection check are all covered; the untestable DOM/visualViewport behavior is honestly disclosed.
  • Security — no concerns; new icons are static s…

✅ Opened pull request: imaustink/glyph#62

@k5s-bot

k5s-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Starting work on this now.

@k5s-bot

k5s-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Watch live or take over the session here: https://claude.ai/code/session_01LvarsPqmJUpqYwcXJgwXRK

@k5s-bot

k5s-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Picked this PR back up under the ai-triage label. Summary of what I did:

Review feedback

The only review on this PR is the automated k5s-bot review, which was a clean approval with no changes requested and no inline review comments. It specifically verified the schema match (LIST_ITEM = 'listItem'), the sinkListItem/liftListItem boolean returns, the position: fixed toolbar context, the cross-platform visualViewport keyboard-inset handling, and the pointerdown+preventDefault blur suppression — all held up. There was therefore no code feedback to address, so no functional changes were made to the feature.

Sync with base

  • Merged the latest main into fix/mobile-list-indent-51 (repo history uses merge commits, and force-push is off the table, so I merged rather than rebased).
  • The merge was clean — no conflicts. The 7 new commits on main touched api/, collab/, SearchModal.svelte, and TaskLinkExtension.*; none overlap this PR's files (Editor.svelte, Icon.svelte, listIndent.ts, listIndent.test.ts).

Verification after the merge

  • pnpm test: 903 tests across 67 files pass (main added 3 TaskLinkExtension tests since this PR was opened; hence 903 vs. the previously reported 900).
  • pnpm check (svelte-check): 0 errors, 0 warnings.
  • Updated the PR description's test count 900 → 903 to match.

Deliberately not done

  • No functional/code changes to the indent-outdent feature — the review requested none, and the task scope is to address feedback and sync, not add unrequested changes.

Note: the pushed merge commit will kick off a fresh CI run; the pre-merge run was already fully green, and the local unit + svelte-check runs above pass, but the new run may still be in progress at the time of this comment.

@k5s-bot

k5s-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Done. The PR is ready to merge. mergeable=MERGEABLE; state=UNSTABLE just reflects the fresh CI run kicked off by the pushed merge commit (the pre-merge run was fully green, and local tests + svelte-check pass). Notably, the merge I made matches this repo's existing convention — PR #61 also synced via Merge remote-tracking branch 'origin/main' into <branch>.

Summary

PR: #62 — fix(editor): add mobile indent/outdent controls for lists (#51)

Feedback found: The only review was the automated k5s-bot review — a clean approval, no changes requested, no inline comments. All CI checks on the PR were already passing. There was therefore no code feedback to act on, and I made no functional changes to the feature (staying within scope).

What I changed:

  • Synced with base — merged latest main into fix/mobile-list-indent-51. Clean, no conflicts (main's 7 new commits touch api/, collab/, SearchModal.svelte, TaskLinkExtension.*; none overlap this PR's files). Used merge (not rebase) to match repo history and because force-push is forbidden.
  • Verified post-merge — pnpm test: 903/903 pass across 67 files; pnpm check (svelte-check): 0 errors, 0 warnings.
  • Updated the PR description — bumped the stale test count 900 → 903 (main added 3 TaskLinkExtension tests since the PR was opened).
  • Posted a summary comment on the PR.

Deliberately not done: No changes to the indent/o…


✅ Opened pull request: imaustink/glyph#62

@k5s-bot k5s-bot Bot removed the ai-triage label Sep 29, 2026
@imaustink
imaustink merged commit 4f090a4 into main Sep 29, 2026
9 checks passed
@imaustink
imaustink deleted the fix/mobile-list-indent-51 branch September 29, 2026 03: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.

Users are unable to indent lists in mobile view

1 participant