refactor(rich-text-editor): refactor rich-text-editor style #2346
Conversation
WalkthroughThe changes in this pull request focus on updating the CSS variable references in the rich text editor's styling files. The Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/theme/src/rich-text-editor/vars.less (1)
4-5: LGTM: Consistent naming and reuse of global variable. Consider updating the comment.The new variable
--tv-RichTextEditor-line-heightfollows the updated naming convention and reuses the global--tv-line-height-numbervariable, promoting consistency.Consider updating the comment "富文本行号" (rich text line number) to "富文本行高" (rich text line height) for clarity, as this variable sets the line height, not a line number.
packages/theme/src/rich-text-editor/index.less (1)
646-646: Consider using a CSS variable for image handle background colorThe background color for the image handle has been set to a hardcoded value
#d2e4ff. To maintain consistency with the rest of the component and improve theming flexibility, consider using a CSS variable instead.Suggestion:
- background: #d2e4ff; + background: var(--tv-RichTextEditor-image-handle-bg-color, #d2e4ff);Don't forget to add this new variable to the
vars.lessfile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/theme/src/rich-text-editor/index.less (8 hunks)
- packages/theme/src/rich-text-editor/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (15)
packages/theme/src/rich-text-editor/vars.less (7)
3-3: LGTM: Consistent naming and reuse of global variable.The new variable
--tv-RichTextEditor-border-colorfollows the updated naming convention and reuses the global--tv-color-bordervariable, promoting consistency across the theme.
6-7: LGTM: Consistent naming and reuse of global variable.The new variable
--tv-RichTextEditor-icon-color-hoverfollows the updated naming convention and reuses the global--tv-color-icon-hovervariable, ensuring consistency with the overall theme for hover states.
8-9: LGTM: Consistent naming and reuse of global variable.The new variable
--tv-RichTextEditor-icon-color-disabledfollows the updated naming convention and reuses the global--tv-color-icon-disabledvariable, ensuring consistency with the overall theme for disabled states.
13-13: LGTM: Consistent naming and reuse of global variable.The new variable
--tv-RichTextEditor-options-box-shadowfollows the updated naming convention and reuses the global--tv-shadow-3-downvariable, ensuring consistency with the overall theme for shadow effects.
15-15: LGTM: Consistent naming and reuse of global variable.The new variable
--tv-RichTextEditor-options-border-radiusfollows the updated naming convention and reuses the global--tv-border-radius-mdvariable, ensuring consistency with the overall theme for border radius values.
16-19: LGTM: Consistent naming and reuse of global variables.The new variables
--tv-RichTextEditor-options-text-colorand--tv-RichTextEditor-options-item-bg-colorfollow the updated naming convention and reuse global variables (--tv-color-textand--tv-color-bg-hoverrespectively). This ensures consistency with the overall theme for text colors and hover background colors.
1-19: Overall, excellent refactoring of rich-text-editor variables.This refactoring significantly improves the consistency and maintainability of the rich-text-editor component. The new variable naming convention (
--tv-RichTextEditor-*) and the extensive use of global theme variables enhance the component's integration with the overall theme system.Key improvements:
- Consistent naming convention across all variables.
- Increased use of global theme variables, promoting better theme cohesion.
- Clear separation of concerns for different aspects of the rich-text-editor (border, icons, options, etc.).
These changes align well with the PR objectives of refactoring the rich-text-editor style. The only minor suggestions are to update a comment for clarity and consider using a global variable for the options background color.
Great work on this refactoring!
packages/theme/src/rich-text-editor/index.less (8)
64-64: LGTM: Updated border color variableThe change from
--ti-rich-text-editor-box-border-colorto--tv-RichTextEditor-border-coloraligns with the new naming convention, improving consistency across the component's styling.
87-87: LGTM: Updated icon color variables for hover and disabled statesThe changes to use
--tv-RichTextEditor-icon-color-hoverand--tv-RichTextEditor-icon-color-disabledprovide more specific control over icon colors in different states, aligning with the new naming convention.Also applies to: 99-99
134-134: LGTM: Updated style variables for options menuThe changes to use
--tv-RichTextEditor-options-bg-color,--tv-RichTextEditor-options-border-radius,--tv-RichTextEditor-options-box-shadow, and--tv-RichTextEditor-line-heightprovide more specific control over various style properties of the options menu, aligning with the new naming convention.Also applies to: 137-138, 145-145
281-281: LGTM: Updated style variables for line height optionsThe changes continue the pattern of using the new
--tv-RichTextEditor-*variables for styling. The addition of--tv-RichTextEditor-options-item-bg-colorand--tv-RichTextEditor-options-text-colorfor hover states provides more granular control over the UI.Also applies to: 284-285, 292-292, 295-296
319-321: LGTM: Consistent updates to font size options stylingThe changes in this segment maintain consistency with the previous updates, using the new
--tv-RichTextEditor-*variables for styling the font size options. This ensures a uniform approach to theming across different parts of the rich text editor.Also applies to: 328-328, 331-332, 338-338
614-614: LGTM: Consistent border color variable updateThe change from
--ti-rich-text-editor-box-border-colorto--tv-RichTextEditor-border-coloris consistent with the earlier update (line 64), ensuring uniform border styling across the rich text editor component.
651-651: LGTM: Consistent border color variable updateThe change from
--ti-rich-text-editor-box-border-colorto--tv-RichTextEditor-border-coloris consistent with the earlier updates (lines 64 and 614), ensuring uniform border styling across the rich text editor component, including the image resize functionality.
Line range hint
1-681: Overall: Consistent and improved styling updatesThe changes in this file successfully implement the new naming convention for CSS variables (
--tv-RichTextEditor-*) throughout the rich text editor component. This update enhances consistency and improves the overall maintainability of the styles. The only minor suggestion is to consider using a CSS variable for the image handle background color (line 646) to maintain full theming flexibility.
| // 头部图标弹出下拉框背景色 | ||
| --ti-rich-text-editor-options-bg-color: var(--ti-common-color-bg-white-normal, #ffffff); | ||
| --tv-RichTextEditor-options-bg-color: #ffffff; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a global variable for the background color.
While the new variable --tv-RichTextEditor-options-bg-color follows the updated naming convention, it uses a hardcoded color value "#ffffff". For better theme flexibility and consistency, consider using a global background color variable instead.
Replace the hardcoded value with a global variable:
- --tv-RichTextEditor-options-bg-color: #ffffff;
+ --tv-RichTextEditor-options-bg-color: var(--tv-color-bg-normal);Ensure that --tv-color-bg-normal or a similar global variable exists in your theme system.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --tv-RichTextEditor-options-bg-color: #ffffff; | |
| --tv-RichTextEditor-options-bg-color: var(--tv-color-bg-normal); |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit