feat(table module): add table property setting #71
Conversation
WalkthroughThe updates introduce enhanced functionalities for managing table properties within a rich text editor. New keys, SVG representations, localization entries, and utility functions are added to improve user interaction and customization of table and cell attributes. These changes collectively aim to enrich the editor's capabilities, providing a more seamless and visually appealing experience for users dealing with tables in their documents. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant Modal
participant TableProperty
participant CellProperty
User->>Editor: Selects table cell
Editor->>CellProperty: Opens modal
CellProperty->>Modal: Displays current properties
User->>Modal: Edits properties
Modal->>CellProperty: Submits changes
CellProperty->>Editor: Updates cell attributes
Editor->>User: Reflects changes in editor
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 as PR comments)
Additionally, you can add 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: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- packages/editor/src/init-default-config/config/hoverbar.ts (1 hunks)
- packages/table-module/src/assets/index.less (1 hunks)
- packages/table-module/src/constants/svg.ts (1 hunks)
- packages/table-module/src/locale/en.ts (2 hunks)
- packages/table-module/src/locale/zh-CN.ts (2 hunks)
- packages/table-module/src/module/custom-types.ts (2 hunks)
- packages/table-module/src/module/index.ts (3 hunks)
- packages/table-module/src/module/menu/CellProperty.ts (1 hunks)
- packages/table-module/src/module/menu/TableProperty.ts (1 hunks)
- packages/table-module/src/module/menu/index.ts (2 hunks)
- packages/table-module/src/module/parse-style-html.ts (1 hunks)
- packages/table-module/src/module/render-style.ts (1 hunks)
- packages/table-module/src/module/style-to-html.ts (1 hunks)
- packages/table-module/src/utils/dom.ts (3 hunks)
- packages/table-module/src/utils/vdom.ts (1 hunks)
Additional context used
Biome
packages/table-module/src/module/parse-style-html.ts
[error] 23-23: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
Additional comments not posted (43)
packages/table-module/src/module/menu/CellProperty.ts (4)
1-5: LGTM!The imports and class extension are appropriate.
7-12: LGTM!The class properties are well-defined and align with the intended functionality.
22-22: LGTM!The export statement is correct.
14-19: LGTM! But verify the usage ofisOfTypeutility function.The method
getModalContentNodeis implemented correctly.However, ensure that the
isOfTypeutility function is correctly implemented and used.Verification successful
The
isOfTypeutility function is correctly implemented and used.The function
isOfTypeis defined inpackages/table-module/src/utils/is-of-type.tsand its implementation aligns with its intended purpose. It correctly checks if a node matches the specified types.
packages/table-module/src/utils/is-of-type.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of `isOfType` utility function. # Test: Search for the implementation of `isOfType`. Expect: Correct implementation and usage. rg --type js -A 5 $'function isOfType'Length of output: 39
Script:
#!/bin/bash # Description: Verify the implementation and usage of `isOfType` utility function. # Search for the definition of `isOfType` function ast-grep --lang javascript --pattern 'function isOfType($_, $_) { $$$ }' # Search for the usage of `isOfType` function rg 'isOfType'Length of output: 2776
Script:
#!/bin/bash # Description: Display the contents of the `is-of-type.ts` file to verify the implementation of `isOfType` function. # Display the contents of `is-of-type.ts` cat packages/table-module/src/utils/is-of-type.tsLength of output: 821
packages/table-module/src/locale/zh-CN.ts (4)
8-14: LGTM!The translations for modal attributes are correct.
15-18: LGTM!The translations for color options are correct.
19-29: LGTM!The translations for border styles are correct.
40-41: LGTM!The translations for table and cell properties are correct.
packages/table-module/src/module/style-to-html.ts (3)
1-4: LGTM!The file header is appropriate.
6-7: LGTM!The imports are appropriate.
8-25: LGTM!The function implementation is correct and handles the conversion of style properties to HTML appropriately.
packages/table-module/src/utils/vdom.ts (1)
21-32: LGTM! Verify the function usage in the codebase.The code changes are approved.
However, ensure that the
addVnodeStylefunction is used correctly throughout the codebase.Verification successful
The
addVnodeStylefunction is used correctly throughout the codebase.The function is utilized in various modules and test files, confirming its proper integration and functionality.
packages/table-module/src/module/render-style.tspackages/basic-modules/src/modules/justify/render-style.tsxpackages/basic-modules/src/modules/line-height/render-style.tsxpackages/basic-modules/src/modules/indent/render-style.tsxpackages/basic-modules/src/modules/color/render-style.tsxpackages/basic-modules/src/modules/font-size-family/render-style.tsxpackages/core/__tests__/utils/vdom.test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `addVnodeStyle` function in the codebase. # Test: Search for the function usage. Expect: Correct usage of `addVnodeStyle`. rg --type js -A 5 $'addVnodeStyle'Length of output: 33
Script:
#!/bin/bash # Description: Verify the usage of the `addVnodeStyle` function in the codebase. # Test: Search for the function usage in both JavaScript and TypeScript files. rg --type-add 'ts:*.ts' --type-add 'tsx:*.tsx' -tjs -tts -ttsx -A 5 $'addVnodeStyle'Length of output: 8603
packages/table-module/src/locale/en.ts (4)
8-14: LGTM!The
modalsection localization entries are appropriate and consistent.
15-18: LGTM!The
colorsection localization entries are appropriate and consistent.
19-29: LGTM!The
borderStylesection localization entries are appropriate and consistent.
40-41: LGTM!The
tablePropertyandcellPropertylocalization entries are appropriate and consistent.packages/table-module/src/module/custom-types.ts (2)
9-16: LGTM!The
TableCellPropertytype definition is correct and consistent.
28-28: LGTM!The
TableCellElementtype definition is correct and consistent.packages/table-module/src/module/render-style.ts (1)
1-4: Ensure the utility functionaddVnodeStyleis correctly implemented.The function
addVnodeStyleis used to add styles to VNodes. Ensure that this utility function correctly handles the style properties.Verification successful
The
addVnodeStylefunction is correctly implemented and handles style properties as expected.The function
addVnodeStyleis consistently implemented across multiple files and correctly handles the style properties by ensuring thatvnode.dataandvnode.data.styleare not null before merging the new styles.
packages/table-module/src/utils/vdom.tspackages/code-highlight/src/utils/vdom.tspackages/core/src/utils/vdom.tspackages/basic-modules/src/utils/vdom.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `addVnodeStyle` function. # Test: Search for the implementation of `addVnodeStyle`. Expect: Correct handling of style properties. rg --type js -A 10 $'function addVnodeStyle'Length of output: 45
Script:
#!/bin/bash # Description: Verify the implementation of the `addVnodeStyle` function. # Test: Search for the implementation of `addVnodeStyle` in JavaScript and TypeScript files. rg --type js --type ts -A 10 'function addVnodeStyle' rg --type js --type ts -A 10 'const addVnodeStyle =' rg --type js --type ts -A 10 'export function addVnodeStyle' rg --type js --type ts -A 10 'export const addVnodeStyle ='Length of output: 4175
packages/editor/src/init-default-config/config/hoverbar.ts (1)
38-40: LGTM! The new keys are correctly integrated.The new keys
setTablePropertyandsetTableCellPropertyare correctly added to themenuKeysarray for thetablekey.packages/table-module/src/module/index.ts (3)
11-13: LGTM! The new imports are correctly integrated.The new imports
renderStyle,styleToHtml, andparseStyleHtmlare correctly added to the file.
26-27: LGTM! The new constants are correctly integrated.The new constants
setTablePropertyConfandsetTableCellPropertyConfare correctly added to themenuconfiguration.
31-33: LGTM! The new properties are correctly integrated.The new properties
renderStyle,styleToHtml,parseStyleHtml,setTablePropertyConf, andsetTableCellPropertyConfare correctly added to thetableconfiguration object.Also applies to: 49-50
packages/table-module/src/assets/index.less (1)
123-196: LGTM! The new styles for modal components are well-structured.The styles enhance the visual structure and user interaction of modal components.
packages/table-module/src/module/menu/TableProperty.ts (6)
37-40: LGTM! ThegetValuemethod is straightforward.The method returns an empty string as expected.
42-45: LGTM! TheisActivemethod is straightforward.The method returns
falseas expected.
47-59: LGTM! TheisDisabledmethod is well-implemented.The method correctly checks if the selection is within a table and returns the appropriate boolean value.
61-65: LGTM! Theexecmethod is a placeholder.The method does not contain any logic as expected.
67-72: LGTM! ThegetModalContentNodemethod is well-implemented.The method correctly retrieves the selected table node using the
Editor.nodesfunction.
80-195: LGTM! ThegetModalContentElemmethod is well-implemented.The method correctly constructs the modal content element, initializes form values, and sets up event handlers.
packages/table-module/src/constants/svg.ts (7)
53-54: LGTM! TheTABLE_PROPERTY_SVGconstant is well-formed.The SVG is suitable for representing table properties.
57-58: LGTM! TheCELL_PROPERTY_SVGconstant is well-formed.The SVG is suitable for representing cell properties.
61-62: LGTM! TheJUSTIFY_LEFT_SVGconstant is well-formed.The SVG is suitable for representing left alignment.
65-66: LGTM! TheJUSTIFY_RIGHT_SVGconstant is well-formed.The SVG is suitable for representing right alignment.
69-70: LGTM! TheJUSTIFY_CENTER_SVGconstant is well-formed.The SVG is suitable for representing center alignment.
73-74: LGTM! TheJUSTIFY_JUSTIFY_SVGconstant is well-formed.The SVG is suitable for representing justified alignment.
77-78: LGTM! TheCLEAN_SVGconstant is well-formed.The SVG is suitable for representing a clean or reset action.
packages/table-module/src/module/menu/index.ts (2)
91-96: LGTM!The configuration for
setTablePropertyConfis consistent with other configurations.
98-103: LGTM!The configuration for
setTableCellPropertyConfis consistent with other configurations.packages/table-module/src/utils/dom.ts (4)
38-38: LGTM!The method
$.fn.datais consistent with other methods.
39-39: LGTM!The method
$.fn.hideis consistent with other methods.
40-40: LGTM!The method
$.fn.showis consistent with other methods.
81-84: LGTM!The function
getOuterHTMLis consistent with other functions.
| export function renderStyle(node: Descendant, vnode: VNode): VNode { | ||
| if (!Element.isElement(node)) return vnode | ||
|
|
||
| const { backgroundColor, borderWidth, borderStyle, borderColor, textAlign } = | ||
| node as TableCellElement | ||
|
|
||
| const props: TableCellProperty = {} | ||
| if (backgroundColor) props.backgroundColor = backgroundColor | ||
| if (borderWidth) props.borderWidth = `${borderWidth}px` | ||
| if (borderStyle) props.borderStyle = borderStyle | ||
| if (borderColor) props.borderColor = borderColor | ||
| if (textAlign) props.textAlign = textAlign | ||
|
|
||
| let styleVnode: VNode = vnode | ||
| if (node.type === 'table') { | ||
| addVnodeStyle((styleVnode.children?.[0] as VNode).children?.[0] as VNode, props) | ||
| } else { | ||
| addVnodeStyle(styleVnode, props) | ||
| } | ||
| return styleVnode |
There was a problem hiding this comment.
Consider refactoring for readability and performance.
The function can be refactored to improve readability and performance by reducing the number of conditional checks and using a single object to store the properties.
export function renderStyle(node: Descendant, vnode: VNode): VNode {
if (!Element.isElement(node)) return vnode
const { backgroundColor, borderWidth, borderStyle, borderColor, textAlign } = node as TableCellElement
const props: TableCellProperty = {
...(backgroundColor && { backgroundColor }),
...(borderWidth && { borderWidth: `${borderWidth}px` }),
...(borderStyle && { borderStyle }),
...(borderColor && { borderColor }),
...(textAlign && { textAlign }),
}
let styleVnode: VNode = vnode
if (node.type === 'table') {
addVnodeStyle((styleVnode.children?.[0] as VNode).children?.[0] as VNode, props)
} else {
addVnodeStyle(styleVnode, props)
}
return styleVnode
}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.
| export function renderStyle(node: Descendant, vnode: VNode): VNode { | |
| if (!Element.isElement(node)) return vnode | |
| const { backgroundColor, borderWidth, borderStyle, borderColor, textAlign } = | |
| node as TableCellElement | |
| const props: TableCellProperty = {} | |
| if (backgroundColor) props.backgroundColor = backgroundColor | |
| if (borderWidth) props.borderWidth = `${borderWidth}px` | |
| if (borderStyle) props.borderStyle = borderStyle | |
| if (borderColor) props.borderColor = borderColor | |
| if (textAlign) props.textAlign = textAlign | |
| let styleVnode: VNode = vnode | |
| if (node.type === 'table') { | |
| addVnodeStyle((styleVnode.children?.[0] as VNode).children?.[0] as VNode, props) | |
| } else { | |
| addVnodeStyle(styleVnode, props) | |
| } | |
| return styleVnode | |
| export function renderStyle(node: Descendant, vnode: VNode): VNode { | |
| if (!Element.isElement(node)) return vnode | |
| const { backgroundColor, borderWidth, borderStyle, borderColor, textAlign } = node as TableCellElement | |
| const props: TableCellProperty = { | |
| ...(backgroundColor && { backgroundColor }), | |
| ...(borderWidth && { borderWidth: `${borderWidth}px` }), | |
| ...(borderStyle && { borderStyle }), | |
| ...(borderColor && { borderColor }), | |
| ...(textAlign && { textAlign }), | |
| } | |
| let styleVnode: VNode = vnode | |
| if (node.type === 'table') { | |
| addVnodeStyle((styleVnode.children?.[0] as VNode).children?.[0] as VNode, props) | |
| } else { | |
| addVnodeStyle(styleVnode, props) | |
| } | |
| return styleVnode | |
| } |
| } | ||
|
|
||
| // eslint-disable-next-line no-unsafe-optional-chaining | ||
| let [borderWidth, borderStyle, borderColor] = getStyleValue($elem, 'border')?.split(' ') |
There was a problem hiding this comment.
Fix unsafe optional chaining usage.
The optional chaining usage here is unsafe and can throw a TypeError. Ensure that getStyleValue($elem, 'border') returns a valid string before using .split(' ').
- let [borderWidth, borderStyle, borderColor] = getStyleValue($elem, 'border')?.split(' ')
+ const border = getStyleValue($elem, 'border')
+ let [borderWidth, borderStyle, borderColor] = border ? border.split(' ') : [undefined, undefined, undefined]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.
| let [borderWidth, borderStyle, borderColor] = getStyleValue($elem, 'border')?.split(' ') | |
| const border = getStyleValue($elem, 'border') | |
| let [borderWidth, borderStyle, borderColor] = border ? border.split(' ') : [undefined, undefined, undefined] |
Tools
Biome
[error] 23-23: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
| export function parseStyleHtml(elem: DOMElement, node: Descendant, editor: IDomEditor): Descendant { | ||
| if (elem.tagName !== 'TABLE' && elem.tagName !== 'TD') return node | ||
|
|
||
| const $elem = $(elem) | ||
|
|
||
| let tableNode = node as TableCellElement | ||
| let backgroundColor = getStyleValue($elem, 'background-color') | ||
| if (!backgroundColor) backgroundColor = getStyleValue($elem, 'background') // word 背景色 | ||
| if (backgroundColor) { | ||
| tableNode.backgroundColor = backgroundColor | ||
| } | ||
|
|
||
| // eslint-disable-next-line no-unsafe-optional-chaining | ||
| let [borderWidth, borderStyle, borderColor] = getStyleValue($elem, 'border')?.split(' ') | ||
| borderWidth = getStyleValue($elem, 'border-width') || borderWidth // border 宽度 | ||
| if (borderWidth) { | ||
| tableNode.borderWidth = borderWidth.replace(/[^\d]/g, '') | ||
| } | ||
| borderStyle = getStyleValue($elem, 'border-style') || borderStyle // border 样式 | ||
| if (borderStyle) { | ||
| tableNode.borderStyle = borderStyle | ||
| } | ||
| borderColor = getStyleValue($elem, 'border-color') || borderColor // border 颜色 | ||
| if (borderColor) { | ||
| tableNode.borderColor = borderColor | ||
| } | ||
|
|
||
| let textAlign = getStyleValue($elem, 'text-align') | ||
| textAlign = getStyleValue($elem, 'text-align') || textAlign // 文本 对齐 | ||
| if (textAlign) { | ||
| tableNode.textAlign = textAlign | ||
| } | ||
| return node |
There was a problem hiding this comment.
Optimize and improve readability.
Consider the following improvements:
- Combine the two
getStyleValue($elem, 'text-align')calls. - Use
constinstead ofletwhere variables are not reassigned. - Use a single
tableNodevariable declaration.
export function parseStyleHtml(elem: DOMElement, node: Descendant, editor: IDomEditor): Descendant {
if (elem.tagName !== 'TABLE' && elem.tagName !== 'TD') return node
const $elem = $(elem)
const tableNode = node as TableCellElement
let backgroundColor = getStyleValue($elem, 'background-color') || getStyleValue($elem, 'background') // word 背景色
if (backgroundColor) {
tableNode.backgroundColor = backgroundColor
}
const border = getStyleValue($elem, 'border')
const [borderWidth, borderStyle, borderColor] = border ? border.split(' ') : [undefined, undefined, undefined]
const finalBorderWidth = getStyleValue($elem, 'border-width') || borderWidth // border 宽度
if (finalBorderWidth) {
tableNode.borderWidth = finalBorderWidth.replace(/[^\d]/g, '')
}
const finalBorderStyle = getStyleValue($elem, 'border-style') || borderStyle // border 样式
if (finalBorderStyle) {
tableNode.borderStyle = finalBorderStyle
}
const finalBorderColor = getStyleValue($elem, 'border-color') || borderColor // border 颜色
if (finalBorderColor) {
tableNode.borderColor = finalBorderColor
}
const textAlign = getStyleValue($elem, 'text-align')
if (textAlign) {
tableNode.textAlign = textAlign
}
return node
}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.
| export function parseStyleHtml(elem: DOMElement, node: Descendant, editor: IDomEditor): Descendant { | |
| if (elem.tagName !== 'TABLE' && elem.tagName !== 'TD') return node | |
| const $elem = $(elem) | |
| let tableNode = node as TableCellElement | |
| let backgroundColor = getStyleValue($elem, 'background-color') | |
| if (!backgroundColor) backgroundColor = getStyleValue($elem, 'background') // word 背景色 | |
| if (backgroundColor) { | |
| tableNode.backgroundColor = backgroundColor | |
| } | |
| // eslint-disable-next-line no-unsafe-optional-chaining | |
| let [borderWidth, borderStyle, borderColor] = getStyleValue($elem, 'border')?.split(' ') | |
| borderWidth = getStyleValue($elem, 'border-width') || borderWidth // border 宽度 | |
| if (borderWidth) { | |
| tableNode.borderWidth = borderWidth.replace(/[^\d]/g, '') | |
| } | |
| borderStyle = getStyleValue($elem, 'border-style') || borderStyle // border 样式 | |
| if (borderStyle) { | |
| tableNode.borderStyle = borderStyle | |
| } | |
| borderColor = getStyleValue($elem, 'border-color') || borderColor // border 颜色 | |
| if (borderColor) { | |
| tableNode.borderColor = borderColor | |
| } | |
| let textAlign = getStyleValue($elem, 'text-align') | |
| textAlign = getStyleValue($elem, 'text-align') || textAlign // 文本 对齐 | |
| if (textAlign) { | |
| tableNode.textAlign = textAlign | |
| } | |
| return node | |
| export function parseStyleHtml(elem: DOMElement, node: Descendant, editor: IDomEditor): Descendant { | |
| if (elem.tagName !== 'TABLE' && elem.tagName !== 'TD') return node | |
| const $elem = $(elem) | |
| const tableNode = node as TableCellElement | |
| let backgroundColor = getStyleValue($elem, 'background-color') || getStyleValue($elem, 'background') // word 背景色 | |
| if (backgroundColor) { | |
| tableNode.backgroundColor = backgroundColor | |
| } | |
| const border = getStyleValue($elem, 'border') | |
| const [borderWidth, borderStyle, borderColor] = border ? border.split(' ') : [undefined, undefined, undefined] | |
| const finalBorderWidth = getStyleValue($elem, 'border-width') || borderWidth // border 宽度 | |
| if (finalBorderWidth) { | |
| tableNode.borderWidth = finalBorderWidth.replace(/[^\d]/g, '') | |
| } | |
| const finalBorderStyle = getStyleValue($elem, 'border-style') || borderStyle // border 样式 | |
| if (finalBorderStyle) { | |
| tableNode.borderStyle = finalBorderStyle | |
| } | |
| const finalBorderColor = getStyleValue($elem, 'border-color') || borderColor // border 颜色 | |
| if (finalBorderColor) { | |
| tableNode.borderColor = finalBorderColor | |
| } | |
| const textAlign = getStyleValue($elem, 'text-align') | |
| if (textAlign) { | |
| tableNode.textAlign = textAlign | |
| } | |
| return node | |
| } |
Tools
Biome
[error] 23-23: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
|
我今晚应该会发一版，后面解决了这个 siblings 的问题再发版。 |
|
这个功能啥时候发布呀 |
还有点bug，得再看看，需要的话方便看下不，这两天得看下另一个bug |
之前没get到，这个是 dom.js 那里漏引入这个fn了～之前赶着上线，所以是独立的文件，合并时漏了这个了 |
添加表格/单元格属性功能
Summary by CodeRabbit
New Features
Bug Fixes
Documentation