Repository navigation
Separate highlights from cards and fix per-keystroke author colors - #78
Merged
Merged
Conversation
Three changes picked out of #77, reimplemented against main. Stop assigning author colors on every keystroke. colorForAuthor ran on every transaction and created + persisted an assignment as a side effect, so typing "Alice" into the Author setting saved generated colors for "A", "Al", "Ali" and "Alic" too. resolveAuthorColor is replaced by a pure readAuthorColor; creation stays at the explicit points that already existed (vault scan, Rescan, "Assign color", load) plus a debounced assignment when a typed Author name settles. Add a Show highlights setting. Highlights used to ride the showComments toggle, so hiding the cards also wiped the underlines out of the text. The two are independent now, and either can be hidden alone. Add "Add comment" to the editor right-click menu when text is selected. It reuses the command's entry point, so the README no longer needs to send people to Commander for it. The vault modify/create/delete/rename listeners and the authorColorsEnabled gate on colorForAuthor are both left intact, unlike in #77.
Table highlights in Live Preview paint through the CSS Custom Highlight API, not `.doc-comment-span`, so the `dc-highlights` class never reaches them and they were still gated on showComments. That inverted both halves of the new setting inside tables: Show highlights off left them painted, and hiding the cards wiped them. Gate them on showHighlights and cover it with a test that fails against the old gate. Carry a saved showComments:false over to showHighlights on first load. Before this branch, hiding the comments hid the highlights too, so defaulting the new setting to true would have switched every highlight back on for those vaults without explanation. Cancel scheduleAuthorColorSave in onunload alongside the new debouncer. The 600ms author-color timer feeds a 100ms save, so unloading in that window let a write land after a reloaded instance had already loaded. Keep the draft marker visible while highlights are off. It is a `.doc-comment-span`, so the blanking rule erased the anchor the open composer is writing against — a state the old code could not reach. Also: add a Toggle highlights command to match the other visibility toggles, make ensureCurrentAuthorColor private, fix comments in styles.css, editor/margin.ts and editor-view.test.ts that still claimed highlights follow showComments, note the Commander duplicate in the README, and add the CHANGELOG entry.
Inheriting a saved showComments:false persisted showHighlights:false for anyone who merely had comments toggled off at update time, and it stuck: turning comments back on later left the highlights gone with no visible cause. Highlights reappearing is immediately visible and one toggle to undo, so take that over a silent sticky one. The test now records the decision rather than the inheritance.
Probe the table-highlight gate with a vi.fn instead of a let flag, type the author name with forEach, and say why the async keystroke test needs a for...of loop.
Mobile has no comment cards, so Toggle comments only ever showed there through the highlights. Once highlights followed Show highlights alone, the ribbon icon and the command said "Comments hidden" on mobile and changed nothing. On mobile, show the highlights only while both settings are on, in Live Preview, Reading view and table cells. Desktop keeps the two settings independent.
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
1 participant
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.
Picks the three usable changes out of #77 and reimplements them against
main. Thanks to @atendev for finding the first two.Stop assigning author colors on every keystroke
colorForAuthorruns on every transaction, and it created and persisted an assignment as a side effect. The Author setting'sonChangefires per keystroke and triggers a refresh, so typingAlicesaved generated colors forA,Al,AliandAlicas well — which is why the Highlight colors list filled up with junk.resolveAuthorColoris replaced by a purereadAuthorColor. The render path can no longer mutate or persist anything.Add a Show highlights setting
Highlights rode the
showCommentstoggle, so hiding the cards also wiped the underlines out of the text. They are independent now — hide the cards and keep the highlights, or the reverse. Applies to editing, reading, and mobile views.Note that with cards hidden, clicking a highlight doesn't do anything yet; the sidebar is how you read those threads. #77's "highlights open sidebar" setting is not included here.
Add "Add comment" to the right-click menu
Appears in the editor context menu when text is selected, reusing the command's entry point. The README no longer sends people to Commander just for this.
Deliberately not included from #77
The author-selector / "Available authors" feature, the action-bar move (it breaks in the sidebar, where cards are
position: static), the more-menu toggle guard, the.dc-actrestyle, the author-colored draft border, and the.gitignorechange.#77 also dropped the
vault.on("modify")→scheduleReadingRefresh()call and theauthorColorsEnabledgate oncolorForAuthor. Both are left intact here.Testing
npm run checkpasses — format, lint, typecheck, 208 tests.Three new tests, each verified to fail against
mainbefore the fix:showHighlights