Remove inline style attributes from Yorkie Tree on clear#304
Merged
Conversation
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 7 minutes and 28 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Docs unlink did nothing because removeLink clears the link via
applyInlineStyle(range, { href: undefined }), but the Yorkie store
only wrote styles with tree.styleByPath, which merges attributes and
can never delete one. serializeInlineStyle also drops undefined keys,
so the clear was silently lost and the href survived on the Tree node
(the CRDT source of truth). The optimistic in-memory cache removed it,
so the link looked gone locally until the next tree re-read and never
disappeared for other collaborators.
applyStyle now computes the attribute names of any InlineStyle key
explicitly set to undefined and calls tree.removeStyleByPath per styled
inline, mirroring the existing setBlockType / updateTableCellSpan
removal pattern. General fix: covers any cleared key, not just href.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
hackerwins
force-pushed
the
fix/docs-unlink-href
branch
from
May 25, 2026 16:16
f53fb0b to
04d1ae9
Compare
Contributor
Verification: verify:selfResult: ✅ PASS in 232.0s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
hackerwins
added a commit
that referenced
this pull request
May 31, 2026
PRs #257/#301/#303/#304/#305/#306/#307/#309/#315/#316/#317/#321/#322 shipped between v0.4.2 and the v0.4.3 cut, leaving their paired todo files in active with the process checkboxes still open. Flip the remaining boxes for the work that landed, then move the paired todo/lessons into docs/tasks/archive/2026/05/ via pnpm tasks:archive. Active count drops from 17 to 6; archive count grows by 21 files. Patch release covering 29 commits since v0.4.2. Headline theme is Slides editing & UX polish toward Google Slides / PowerPoint parity: smart guides (equal-spacing / distance / size), Shift drag modifiers, text inside shapes, a Format options right panel, tier-1 universal toolbar controls, text autofit (shrink + grow), group-selection visuals, and several smaller text-box / selection fixes. PPTX import gained paragraph-level bullet styles, body anchor (vertical alignment), and stretched blipFill cover-crop. Docs added a pending-inline-style at collapsed caret, a caret that tracks the resolved text color, bullet indent on Tab / Shift+Tab, font / size / line-spacing / clear formatting toolbar controls, an inline-style-attribute clear that actually lands in the CRDT, and a polished toolbar trigger / color swatch pass shared across docs / sheets / slides. Sheets gained .xlsx import. Infrastructure: Docker images now build on native arm64 runners, fixing the release hang seen on v0.4.2. No Prisma migration, no new backend env vars — the devops bump is a routine image-tag change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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.
Summary
Clicking unlink in the docs editor did nothing — the hyperlink stayed.
removeLinkclears the link viaapplyInlineStyle(range, { href: undefined }), but the Yorkie store (yorkie-doc-store.ts) only wrote styles withtree.styleByPath, which merges attributes and can never delete one.serializeInlineStylealso dropsundefinedkeys, so the clear was silently lost and thehrefsurvived on the Tree node (the CRDT source of truth). The optimistic in-memory cache removed it, so the link looked gone locally until the next tree re-read (remote change / reload) — and never disappeared for other collaborators.This is exactly the gotcha already handled in
setBlockTypeandupdateTableCellSpan("styleByPath merges, not replaces" → useremoveStyleByPath), but the inlineapplyStylepath missed it.Fix
applyStylenow computes the Yorkie attribute names of anyInlineStylekey explicitly set toundefined(removedInlineStyleAttrs) and callstree.removeStyleByPathper styled inline, alongside the existingstyleByPathmerge. The inline path range matches the established single-node removal pattern. General fix: covers any cleared key, not justhref.Test plan
tests/app/docs/yorkie-doc-store.test.ts("applyStyle attribute removal"):hreffrom the Tree whenapplyStylegets{ href: undefined }hrefis clearedYorkieDocStoreover the same doc), not the optimistic cachepnpm verify:fastgreen (lint + all unit tests); pre-pushverify:self(all build lanes + entropy) green.🤖 Generated with Claude Code