feat(editor): DP-175912 extend table extension for table styling#1130
feat(editor): DP-175912 extend table extension for table styling#1130Maysam Kangarani Farahani (maysamkf) wants to merge 7 commits intostagingfrom
Conversation
| border: var(--dt-size-border-100) solid var(--dt-color-foreground-primary); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
removing td styling when allowCustomTables is enabled
|
Please add either the |
Maysam Kangarani Farahani (maysamkf)
added
the
no-visual-test
Maysam Kangarani Farahani (maysamkf)
marked this pull request as ready for review
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 499b4dba88
ℹ️ 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 (@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 (@codex) address that feedback".
| &--custom-tables > .ProseMirror { | ||
| td { | ||
| padding-left: unset; | ||
| border: unset; | ||
| } |
There was a problem hiding this comment.
When allowCustomTables is enabled, this block only unsets td border/padding, but the base rule in the same file still forces table { border-collapse: collapse; }. That means pasted tables relying on cellspacing/separate borders (a key custom-table use case) still render collapsed inside the editor, so users won’t see preserved spacing while editing even though attributes are kept in HTML output. Add a table override under the custom-tables modifier to avoid inheriting the default collapsed layout.
Useful? React with 👍 / 👎.
|
/windsurf-review |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Other comments (2)
-
packages/dialtone-vue/components/rich_text_editor/rich_text_editor.vue (965-965)
Inconsistent spacing in the Table configuration compared to CustomTable configuration.
extensions.push(Table.configure({ resizable: true }), TableRow, TableHeader, TableCell, Gapcursor); - packages/dialtone-vue/components/rich_text_editor/rich_text_editor.test.js (835-898) There's inconsistent indentation throughout the newly added tests. For example, in the test 'should preserve complex email signature table with mixed attributes when pasted', some lines use 2-space indentation while others use more. Consider running a formatter on this file to ensure consistent indentation.
💡 To request another review, post a new comment with "/windsurf-review".
| border: { | ||
| default: null, | ||
| parseHTML: element => element.getAttribute('border'), | ||
| renderHTML: attributes => { | ||
| if (!attributes.border) return {}; | ||
| return { border: attributes.border }; | ||
| }, | ||
| }, | ||
| cellpadding: { | ||
| default: null, | ||
| parseHTML: element => element.getAttribute('cellpadding'), | ||
| renderHTML: attributes => { | ||
| if (!attributes.cellpadding) return {}; | ||
| return { cellpadding: attributes.cellpadding }; | ||
| }, | ||
| }, | ||
| cellspacing: { | ||
| default: null, | ||
| parseHTML: element => element.getAttribute('cellspacing'), | ||
| renderHTML: attributes => { | ||
| if (!attributes.cellspacing) return {}; | ||
| return { cellspacing: attributes.cellspacing }; | ||
| }, | ||
| }, | ||
| style: { | ||
| default: null, | ||
| parseHTML: element => element.getAttribute('style'), | ||
| renderHTML: attributes => { | ||
| if (!attributes.style) return {}; | ||
| return { style: attributes.style }; | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Consider creating a helper function to generate attribute configurations to reduce code duplication. The same pattern is repeated for multiple attributes (border, cellpadding, cellspacing, style, etc.).
For example:
const createAttribute = (name) => ({
default: null,
parseHTML: element => element.getAttribute(name),
renderHTML: attributes => {
if (!attributes[name]) return {};
return { [name]: attributes[name] };
},
});
// Then use it like:
// border: createAttribute('border'),
// cellpadding: createAttribute('cellpadding'),
// etc.| clipboardData.setData('text/html', | ||
| '<table border="0" cellpadding="0" cellspacing="0" style="border-collapse: collapse;">' + | ||
| '<tr style="background-color: #ffffff;">' + | ||
| '<td style="padding: 5px;" valign="top" width="300">' + | ||
| '<div style="font-size:14pt; color:#333333; font-weight:bold;">John Doe</div>' + | ||
| '</td>' + | ||
| '</tr>' + | ||
| '<tr>' + | ||
| '<td style="padding: 5px;" valign="top" width="300">' + | ||
| '<div style="font-size:10pt; color:#666666;">Support Team</div>' + | ||
| '</td>' + | ||
| '</tr>' + | ||
| '</table>', | ||
| ); |
There was a problem hiding this comment.
The HTML string in the 'should preserve complex email signature table with mixed attributes when pasted' test is quite long and makes the test harder to read. Consider extracting this HTML to a constant at the top of the file or to a helper function that generates test HTML.
There was a problem hiding this comment.
imo i think this is fine, but lmk if anyone thinks this should be changed.
There was a problem hiding this comment.
yeah I'm okay with it
packages/dialtone-vue/components/rich_text_editor/extensions/table/table.js
Outdated
Show resolved
Hide resolved
…able/table.js Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com>
…ltone into extend-table-support
|
✔️ Deploy previews ready! |
3 participants
PR Title
Obligatory GIF (super important!)
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
https://dialpad.atlassian.net/browse/DP-175912
📖 Description
allowCustomTablesprop💡 Context
The Omnichannel team is using the rich text editor to support editing signatures in email channels.
Requirement to support some signatures with custom table styling
📝 Checklist
For all PRs:
For all Vue changes:
📷 Screenshots / GIFs
Old:
New: