fix: update marker on font change#3596
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cfbdb4ca7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
cubic analysis
2 issues found across 4 files
Linked issue analysis
Linked issue: SD-3238: Bug: Existing list markers do not restyle after changing font family or size
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Existing list markers restyle immediately when the user changes font family on an existing list selection. | The new syncListMarkerFontFromParagraphRuns copies resolved run/paragraph fontFamily into the marker run (unless numbering explicitly defines the marker family). A behavior test was added that selects an ordered list, sets fontFamily to Georgia, and asserts the marker's computed fontFamily contains Georgia. |
| ✅ | Existing list markers restyle immediately when the user changes font size on an existing list selection. | The sync logic also updates markerRun.fontSize from paragraph/run fontSize. A behavior test was added that changes font size on a bullet list and asserts the marker's computed fontSize increased accordingly. |
| ✅ | Marker restyling applies consistently for both bulleted and numbered lists. | Behavior tests include assertions for both ordered (numbered) and bullet lists. Unit tests also include a Symbol/font-family scenario and preserve numbering-defined marker font family while syncing size, covering both marker types and numbering-defined fonts. |
| ✅ | The behavior is covered by automated tests that would fail on the current implementation. | A dedicated unit test file and updated behavior tests were added to assert the new marker restyling behavior; these tests encode the expected outcomes described in the issue. |
| ✅ | The fix does not regress the existing behavior where newly created list items inherit the surrounding list font styling. | The behavior test asserting that a newly created empty list item inherits the previous paragraph's font was preserved and updated; the sync implementation preserves numbering-defined fontFamily and only overrides when appropriate, and the integration calls occur in paragraph conversion/handling to avoid breaking inheritance. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
cd68e21 to
10b9bec
Compare
No description provided.