fix(editor): persist text alignment as a sidecar marker in the markdown - #1947
Draft
h4yfans wants to merge 9 commits into
Draft
fix(editor): persist text alignment as a sidecar marker in the markdown#1947h4yfans wants to merge 9 commits into
h4yfans wants to merge 9 commits into
Conversation
Block colours, table cell colours and, from here, text alignment all live in HTML comment lines directly above the block they describe, because GFM has no syntax for any of them. One registry fixes their on-disk order and turns each line into a patch on the next block, so a serializer reads and writes every marker through two calls instead of one pending slot per marker. `left` is never written and never matched, so a vault nobody has aligned keeps its exact bytes.
Red on both serializers: a centred paragraph serializes without its alignment and parses back left-aligned. Also pins what the main converter already does with an HTML comment it does not own: the line is dropped, and `<!-- align:left -->` stays on that path.
…lizers The toolbar set `textAlignment` on the block and neither serializer wrote it, so centre and right reverted to left on the next open. Both serializers now read and write every sidecar marker through the shared registry: the two colours branches become one `parseSidecarMarkerLine` branch with a single pending patch list, and `sidecarMarkerLines` replaces the local `colorMarkerLines`. `textAlignment` is one of BlockNote's default props, so ProseMirror keeps it across the CRDT hop without a schema change. Closes #1937
…vault Three corpus cases run through both serializers, and a golden fixture runs through the frontmatter byte gate and the CRDT converter. The mobile editor asset is regenerated because the corpus lives in packages/editor-schema.
Centres a paragraph and right-aligns a heading through the live editor, waits for both markers to reach the file, then reads the props back after a tab switch and again in a fresh app launched against the same vault.
The calendar widget refresh suite faked the clock onto 2026-08-31, the day it was written. use-today snapshots the local date into module scope at import and re-reads the wall clock for its first subscriber, so on every later day that fake clock arrives as a midnight rollover. todayCalendarRange moves, the useCalendarRange query key moves with it, and the widget fetches a second day during mount, which is the second getRange call the first test counted. Derive the fixture clock and its event hours from the real local date instead, so the mount no longer straddles a day boundary. Local date fields rather than a UTC instant, because far enough from UTC the two name different days.
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Main has no callout branch, so a centred callout reached its marker branch and wrote `<!-- align:center -->` above `> [!info]`; the renderer's callout branch ran first and wrote nothing, and its blockquote splitter treated the marker line above a run as text, which both declined the callout claim and dropped the pending marker. Reopening main's bytes on the renderer gave a centred quote, and the next save deleted the marker. `splitMarkdownByBlockquoteRuns` now treats sidecar marker lines as part of the block below them, lifts them onto the claimed run, and `parseMarkdownWithoutToggles` applies them to the callout or quote (or re-prefixes them when the claim is declined). The renderer's callout branch writes the markers, so both serializers agree. A centred and a coloured callout are pinned in the shared corpus for both pipelines; the coloured one was broken the same way before this change.
… path The example was fenced as markdown, so prettier split the marker from its paragraph with a blank line, which is not the form the app writes.
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
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.
Summary
Centre and right alignment reverted to left on the next open (#1937).
TextAlignButtonsettextAlignmenton the block, and neither serializer wrote it: GFM has no alignment syntax, so the prop was dropped on save and every block came back with the default.The alignment now lives in the markdown file, as a comment line directly above the block, the same way block colours and the toggle's
openattribute already do:<!-- align:center --> Centred paragraphcenter,rightandjustifyare the only values written and the only values matched.leftis the default and writes nothing, so every note already in a vault keeps its exact bytes, and a note written by an older version parses as it does today. A hand-written<!-- align:left -->or any other HTML comment is not claimed and stays on the path the parser already gives an unrecognised comment (the main converter drops it; that behaviour is now pinned, not changed). The marker attaches to the next block only. Headings carrytextAlignmenttoo and are covered. A toggle's own alignment is not written; the toggle region is split off before the marker scanner runs, and that is documented as a limit. Two more edges, both shared with block colours: a list item whose children need nesting markers is serialized alone when it carries a marker (the marker branch sits ahead of the nesting-children path on both sides, so those children flatten; reordering would drop the parent's marker instead, so it is disclosed here rather than moved), and a callout under a marker used to be readable only by the main parser. That second one is fixed in this PR: the renderer's blockquote splitter (splitMarkdownByBlockquoteRuns) treated a comment line above> [!info]as text, which both declined the callout claim and dropped the pending marker, so main wrote<!-- align:center -->above a centred callout and the renderer reopened it as a centred quote. The splitter now treats marker lines as part of the block below, lifts them onto the claimed run, andparseMarkdownWithoutTogglesapplies them; the renderer's callout branch writes the marker too, so the two serializers agree. A centred callout and a coloured callout are pinned in the shared corpus for both pipelines. The file is the sync payload, so a second device gets the alignment with the note.Scope.
packages/shared/src/block-markers.tsis new: one registry of sidecar markers (MARKERS_IN_DISK_ORDER: colours, table colours, alignment) that both serializers read throughparseSidecarMarkerLineand write throughsidecarMarkerLines. A marker line becomes aSidecarPatchapplied to the block that follows. Inapps/desktop/src/renderer/src/components/note/content-area/markdown-utils.tsandapps/desktop/src/main/sync/blocknote-converter.tsthe two colours branches collapse into that one branch with a single pending patch list, and the localcolorMarkerLinesfunctions go.block-colors.tsis untouched;serializeTogglestill uses it directly.textAlignmentis one of BlockNote'sdefaultProps, so ProseMirror keeps it across the CRDT hop and no editor-schema propSchema change was needed; the CRDT round-trip test proves that rather than assuming it. The round-trip corpus inpackages/editor-schema/src/conformance.tsgains three cases, which is whyapps/mobile/src/editor/generated/editor-web-asset.tsis regenerated.Blast radius. Every note save on both paths goes through the registry now, including notes with block or cell colours. The colours bytes are unchanged (their tests are untouched and green), and a note with no coloured or aligned block emits no marker at all. Main's marker branch keeps its deliberately unguarded fence posture, same as the colours branch it replaces.
Carries the test-only main fix from the calendar-widget PR until it lands; rebasing after that drops it.
Stack. This is the first of two PRs from one decision: #1936 (table column widths) builds on the registry and is opened against this branch.
Closes #1937
Release note
Centre, right and justified text alignment now stays put when you leave a note and when you restart the app.
Test plan
Unit, red first. The renderer suite
text alignment markers (#1937)failed 3 of 7 against the untouched serializers (expected 'Intro\nCentred\nRanged' to contain '<!-- align:center -->\nCentred'), and the main suitetext alignment survives the markdown round trip (#1937)failed 8 of 10 (props.textAlignmentcame back'left', and the CRDT path returned'Centred'without its marker). Both green with the fix.Callout follow-up (audit finding on the first head): renderer
markdown-utils.test.ts36 passed with the two new callout cases, rendererroundtrip-conformance.test.ts71 passed withcentred calloutandcoloured calloutadded, mainblocknote-converter.test.ts283 passed with a centred-callout round trip, mainblocknote-converter.roundtrip.test.ts80 passed. Red first: withcallout-block.tsxandmarkdown-utils.tschecked out from the previous head44ffb1218and the corpus at HEAD, the renderer corpus fails oncentred callout(1 failed, 70 passed); restored to HEAD it passes. Thecoloured calloutbytes happened to survive on the old code because the declined callout re-parsed as a quote that serializes to the same text, so that case pins the block type only through the parser unit test; the bytes were never the tell there.Mutation check against the staged fix, six mutations, all killed: the align marker never written (shared 2, renderer 2, main 5 red),
rightdropped from the regex (shared 2, main 2), the patch writingleft(shared 1, renderer 2, main 5), the renderer replacing the pending list instead of stacking (renderer 1), main clearing the pending list without applying it (main 12), and a reviewer-style stale-identity mutation handing the cleared array to the next block (renderer 4).Gates, all from the worktree root on Node 24:
pnpm --filter @memry/desktop typecheck:webpass (28 s),typecheck:nodepass (16 s),typecheck:testpass (51 s),pnpm --filter @memry/shared typecheckpass,pnpm --filter @memry/editor-schema typecheckandtestpass,pnpm lintpass,pnpm --filter @memry/desktop i18n:checkpass,git diff --checkclean,pnpm check:architectureandpnpm check:contractspass,pnpm --filter @memry/desktop test:renderer8639 passed (2 expected fail, 7 skipped),pnpm --filter @memry/desktop test:main7750 passed (1 expected fail, 6 skipped) afterensure-native.sh node,pnpm docs:impact --base origin/main --strictcovered,pnpm docs:buildpass,node apps/mobile/scripts/build-editor-web.mjs --checkcurrent.ipc:checknot run: no contract or IPC handler changed.E2E.
tests/e2e/text-alignment-persist.e2e.tscentres a paragraph and right-aligns a heading throughwindow.__memryEditor.updateBlock, pollsnotes.getuntil both marker lines are in the file body, reopens the note after a tab switch, then launches a second app against the same vault and asserts the props and.bn-block-content[data-text-alignment="center"]. Run under the E2E lock together withnote-open-byte-stability.e2e.tsandforeign-markdown-roundtrip.e2e.ts: 8 passed in 1.6 min, this spec in 17.9 s. Then with only the two serializers checked out from origin/main and every other file at HEAD, the same spec fails at the file-body poll (expect(received).toContain(expected)at the<!-- align:center -->snippet, 38 s), so the marker is what the fix adds; the serializers were restored to HEAD afterwards and the tree matched HEAD again. The renderer save path is what this spec drives (no sync session, somarkdown-utils.tswrites the file); the main path is covered by the CRDT round-trip unit test above.