refactor(editor): narrow text format parameter #12946
Conversation
WalkthroughThe changes restructure text attribute types by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Toolbar
participant Command
participant Types
UI->>Toolbar: User triggers text style/highlight change
Toolbar->>Command: Call command with AffineTextStyleAttributes
Command->>Types: Use AffineTextStyleAttributes for style, AffineTextAttributes for full attributes
Command-->>Toolbar: Return updated attributes
Toolbar-->>UI: Update UI with new styles/attributes
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
blocksuite/affine/inlines/preset/src/command/utils.ts (1)
80-81: Address the static analysis hint about void in union type.The static analysis tool correctly identifies that
voidin union types can be confusing. Consider usingundefinedinstead for better clarity.) => { textAttributes: AffineTextAttributes } | boolean | void + ) => { textAttributes: AffineTextAttributes } | boolean | undefinedblocksuite/affine/inlines/preset/src/command/text-style.ts (1)
75-78: Address the static analysis hint about empty object type.The static analysis tool correctly identifies that
{}is too permissive. Consider using a more specific type orRecord<string, never>for an empty object.-export const getTextAttributes: Command< - {}, - { textAttributes: AffineTextAttributes } -> = (ctx, next) => { +export const getTextAttributes: Command< + Record<string, never>, + { textAttributes: AffineTextAttributes } +> = (ctx, next) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
blocksuite/affine/blocks/root/src/configs/toolbar.ts(2 hunks)blocksuite/affine/components/src/highlight-dropdown-menu/dropdown-menu.ts(5 hunks)blocksuite/affine/inlines/preset/src/command/config.ts(7 hunks)blocksuite/affine/inlines/preset/src/command/format-text.ts(2 hunks)blocksuite/affine/inlines/preset/src/command/index.ts(1 hunks)blocksuite/affine/inlines/preset/src/command/text-style.ts(3 hunks)blocksuite/affine/inlines/preset/src/command/utils.ts(2 hunks)blocksuite/affine/shared/src/types/index.ts(1 hunks)blocksuite/affine/widgets/keyboard-toolbar/src/config.ts(11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
blocksuite/affine/inlines/preset/src/command/config.ts (1)
blocksuite/affine/inlines/preset/src/command/text-style.ts (1)
isTextAttributeActive(89-110)
blocksuite/affine/inlines/preset/src/command/format-text.ts (1)
blocksuite/affine/shared/src/types/index.ts (1)
AffineTextStyleAttributes(38-46)
blocksuite/affine/blocks/root/src/configs/toolbar.ts (1)
blocksuite/affine/components/src/highlight-dropdown-menu/dropdown-menu.ts (1)
HighlightType(23-26)
blocksuite/affine/widgets/keyboard-toolbar/src/config.ts (3)
blocksuite/affine/inlines/preset/src/command/text-style.ts (1)
getTextAttributes(75-87)blocksuite/affine/widgets/keyboard-toolbar/src/icons.ts (1)
HighLightDuotoneIcon(31-49)blocksuite/affine/shared/src/types/index.ts (1)
AffineTextStyleAttributes(38-46)
blocksuite/affine/shared/src/types/index.ts (1)
blocksuite/affine/model/src/consts/doc.ts (2)
ReferenceInfo(58-58)FootNote(93-93)
blocksuite/affine/components/src/highlight-dropdown-menu/dropdown-menu.ts (1)
blocksuite/affine/shared/src/types/index.ts (1)
AffineTextStyleAttributes(38-46)
blocksuite/affine/inlines/preset/src/command/utils.ts (2)
blocksuite/affine/shared/src/types/index.ts (1)
AffineTextAttributes(48-61)blocksuite/affine/shared/src/services/toolbar-service/context.ts (1)
chain(33-35)
blocksuite/affine/inlines/preset/src/command/text-style.ts (3)
blocksuite/affine/inlines/preset/src/command/index.ts (3)
toggleTextStyleCommand(19-19)isTextAttributeActive(14-14)getTextAttributes(13-13)blocksuite/affine/shared/src/types/index.ts (2)
AffineTextStyleAttributes(38-46)AffineTextAttributes(48-61)blocksuite/affine/inlines/preset/src/command/utils.ts (1)
getCombinedTextAttributes(177-202)
🪛 Biome (1.9.4)
blocksuite/affine/inlines/preset/src/command/utils.ts
[error] 80-80: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
blocksuite/affine/inlines/preset/src/command/text-style.ts
[error] 76-76: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (53)
- GitHub Check: Unit Test (5)
- GitHub Check: Unit Test (4)
- GitHub Check: Unit Test (2)
- GitHub Check: Unit Test (3)
- GitHub Check: Unit Test (1)
- GitHub Check: Native Unit Test
- GitHub Check: E2E BlockSuite Cross Browser Test (2, webkit)
- GitHub Check: E2E Test (8)
- GitHub Check: E2E Test (2)
- GitHub Check: E2E Test (5)
- GitHub Check: E2E Test (10)
- GitHub Check: E2E Test (7)
- GitHub Check: E2E Test (9)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, firefox)
- GitHub Check: E2E Test (6)
- GitHub Check: E2E Test (4)
- GitHub Check: E2E Test (3)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, chromium)
- GitHub Check: E2E Test (1)
- GitHub Check: Build Server native
- GitHub Check: E2E BlockSuite Cross Browser Test (2, firefox)
- GitHub Check: E2E BlockSuite Test (2)
- GitHub Check: E2E BlockSuite Test (10)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, webkit)
- GitHub Check: E2E BlockSuite Test (1)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, chromium)
- GitHub Check: E2E BlockSuite Test (7)
- GitHub Check: E2E BlockSuite Test (6)
- GitHub Check: E2E BlockSuite Test (9)
- GitHub Check: Run native tests
- GitHub Check: E2E BlockSuite Test (3)
- GitHub Check: E2E BlockSuite Test (8)
- GitHub Check: E2E BlockSuite Test (5)
- GitHub Check: E2E BlockSuite Test (4)
- GitHub Check: E2E Mobile Test (4)
- GitHub Check: E2E Mobile Test (3)
- GitHub Check: E2E Mobile Test (5)
- GitHub Check: fuzzing
- GitHub Check: E2E Mobile Test (2)
- GitHub Check: y-octo binding test on x86_64-pc-windows-msvc
- GitHub Check: y-octo binding test on x86_64-apple-darwin
- GitHub Check: Build AFFiNE native (aarch64-pc-windows-msvc)
- GitHub Check: E2E Mobile Test (1)
- GitHub Check: Build AFFiNE native (x86_64-pc-windows-msvc)
- GitHub Check: y-octo binding test on aarch64-pc-windows-msvc
- GitHub Check: loom thread test
- GitHub Check: Analyze (typescript, blocksuite)
- GitHub Check: Analyze (typescript, affine)
- GitHub Check: Build @affine/electron renderer
- GitHub Check: Analyze (javascript, blocksuite)
- GitHub Check: Analyze (javascript, affine)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (18)
blocksuite/affine/shared/src/types/index.ts (1)
38-61: Excellent type refactoring for better separation of concerns.The introduction of
AffineTextStyleAttributesfor style-specific properties and the refactoring ofAffineTextAttributesas an intersection type is a clean architectural improvement. This separation allows components to work with only the attributes they need (e.g., formatting commands with style attributes only) while maintaining backward compatibility.blocksuite/affine/inlines/preset/src/command/config.ts (2)
14-14: LGTM: Consistent import update.The import correctly reflects the renamed command from
isTextStyleActivetoisTextAttributeActive.
41-121: LGTM: All usages consistently updated.All text format configurations properly use the renamed
isTextAttributeActivecommand. The updates are consistent across all format types (bold, italic, underline, strike, code, link).blocksuite/affine/inlines/preset/src/command/format-text.ts (2)
3-6: LGTM: Added necessary import for type narrowing.The import of
AffineTextStyleAttributesis correctly added to support the parameter type change.
19-19: Excellent type narrowing for better semantic correctness.Changing the
stylesparameter type fromAffineTextAttributestoAffineTextStyleAttributesis semantically correct. The format text command should only accept style attributes (bold, italic, color, background, etc.) and not metadata attributes (links, references, footnotes, etc.). This improves type safety and prevents misuse.blocksuite/affine/blocks/root/src/configs/toolbar.ts (2)
12-12: LGTM: Import updated for more precise typing.The import change to use
HighlightTypealigns with the type narrowing for the highlight functionality.
143-143: Perfect type narrowing for highlight functionality.Changing the parameter type from
AffineTextAttributestoHighlightTypeis semantically correct. The highlight function should only accept color and background properties, andHighlightType(defined asPick<AffineTextStyleAttributes, 'color' | 'background'>) provides exactly that precision.blocksuite/affine/inlines/preset/src/command/index.ts (1)
13-14: LGTM: Consistent export renaming.The exports are correctly renamed from
getTextStyle→getTextAttributesandisTextStyleActive→isTextAttributeActive, maintaining consistency with the broader refactor to use "text attributes" terminology.blocksuite/affine/widgets/keyboard-toolbar/src/config.ts (3)
29-29: LGTM: Consistent import updates align with the refactor.The import changes from
getTextStyletogetTextAttributesand fromAffineTextAttributestoAffineTextStyleAttributesare well-coordinated and improve type specificity for style-related operations.Also applies to: 48-48
820-821: LGTM: Variable renaming is consistent across all text style operations.The consistent renaming from
textStyletotextAttributesmaintains clarity and aligns with the broader refactoring effort throughout the codebase.Also applies to: 831-832, 842-843, 853-854, 864-865, 875-876, 886-888
919-919: LGTM: Type narrowing improves type safety.Using
AffineTextStyleAttributesinstead ofAffineTextAttributesfor style payloads is a good improvement that provides more specific typing for style-only operations.Also applies to: 964-964
blocksuite/affine/inlines/preset/src/command/utils.ts (1)
177-202: LGTM: Function renaming is consistent with the refactor.The renaming from
getCombinedTextStyletogetCombinedTextAttributesand the property updates fromtextStyletotextAttributesalign well with the broader refactoring effort.blocksuite/affine/components/src/highlight-dropdown-menu/dropdown-menu.ts (3)
1-1: Excellent type refinement improves API design.The import change to
AffineTextStyleAttributesand the redefinition ofHighlightTypeas aPicktype provides better type safety and clearly communicates which properties are relevant for highlighting operations.Also applies to: 23-26
39-39: LGTM: Simplified API design with single parameter.The refactor from separate
valueandtypeparameters to a singlestyleobject parameter simplifies the API and makes it more maintainable.Also applies to: 41-45
77-77: LGTM: Event handlers updated to use object syntax.The event handlers now correctly pass style objects with
{ color: value }and{ background: value }syntax, which aligns with the new API design.Also applies to: 98-98
blocksuite/affine/inlines/preset/src/command/text-style.ts (3)
5-8: LGTM: Import updates support the type refactoring.The addition of both
AffineTextAttributesandAffineTextStyleAttributesimports and the function rename fromgetCombinedTextStyletogetCombinedTextAttributesare consistent with the broader refactor.Also applies to: 14-14
16-21: LGTM: Type narrowing for style-specific operations.Using
AffineTextStyleAttributesfor style toggle operations is a good improvement that provides more specific typing and better type safety.Also applies to: 53-57
75-87: LGTM: Command renaming and implementation updates are consistent.The renaming from
getTextStyletogetTextAttributesandisTextStyleActivetoisTextAttributeActive, along with the updated property access patterns, maintains consistency across the refactor.Also applies to: 89-110
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## canary #12946 +/- ##
==========================================
+ Coverage 55.93% 56.23% +0.29%
==========================================
Files 2659 2669 +10
Lines 127378 127727 +349
Branches 20068 20127 +59
==========================================
+ Hits 71252 71822 +570
+ Misses 53791 53736 -55
+ Partials 2335 2169 -166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR Dependency Tree
This tree was auto-generated by Charcoal
Summary by CodeRabbit
Refactor
New Features
Bug Fixes