-
-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Desktop: Fixes #10562: Preserve table customization made on RTE #10927
base: dev
Are you sure you want to change the base?
Conversation
// Deskotp app table have, by default, border-collapse style, | ||
// so we need one more to know if the table is modified | ||
const isModifiedRTETable = (tableNode) => { | ||
return tableNode.style.length > 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, this could break if the renderer package is updated to apply different default styles to tables. Here are some ideas to prevent regressions:
- Add a test that verifies that a table rendered with the latest version of the renderer package is converted back to Markdown (with
preserveNestedTables: true
). - Would it be possible to check for the presence of specific style attributes? For example, convert to HTML if
['background-color', 'border-color', 'border']
includes one of the style properties.
Thank you for working on this!
We might want to rethink the way the "resizing check" is implemented: The issue is that the only way to know if the column/row was resized is by checking each column and each row and comparing its values. |
wasResized(tableNode) | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had this function - tableShouldBeHtml()
which clearly tells if a table should be html or not. Wasn't it possible to extend this function?
Fixes #10562
Summary
Tables created on RTE and modified would lose its configuration when the user opened the note on Markdown editor.
Now the values should be kept when the user toggles between the two editors.
To fix this I'm checking if some style properties exist that modify the table:
background-color
: changes the background color of the cellfloat
,margin-left
,margin-right
: changes the alignment of the tableI'm ignoring these properties (changing them doesn't preserve the table):
width
,height
: I think if we preserve the HTML table if they are modified we would have to convert even simple tables.border
,cell spacing
,cell padding
: desktop has a default style that overrides the one set in the options, so even if set there is no visual change (should this be changed?)Testing
I added some tests on app-cli, trying to follow the pattern we already have there.