-
Notifications
You must be signed in to change notification settings - Fork 25
feat: Insert markdown tables #816
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
Conversation
49e56a9 to
325dde4
Compare
|
Reusing text here is an interesting approach. There's probably a number of ways to turn the table into something else. But that's not something you want to protect against, or do you? To make sure I understand what you're aiming for here... It's basically a rich text entry that starts out as a table but could turn into anything? |
|
Very interesting approach with the html2canvas to render it. I think this is a good way to get this implemented for now.
Would they not work due to the transformation to render on the canvas? In that case maybe limiting it to tables may be a good idea.
I'd say for the current scope yes. From a user perspective inline editing would be the nicest, but that comes with a much higher complexity.
Unsure about this one, to start with we could store a user locking the editing in the custom data for now maybe? Also has potential to fail but may be a good first step. Collaborative editing of tables is out of scope for now, but we should see if we can avoid at least obvious editing conflict scenarios. |
marcoambrosini
left a comment
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.
Really nice work @silverkszlo :)
A few comments from my side:
- I think that the best place for the insert action is the toolbar itself, together with all the other insert actions
- Cells should be empty of text upon creation
- We should remove all borders in the dialog, including from the text toolbar. The table’s own border are enough to create separation between the elements
- We should also remove the padding left and right of the table
Here’s a screenshot of these changes I did by fiddling with the inspector
325dde4 to
2b4566f
Compare
f888d50 to
e629666
Compare
|
Okay so the html2canvas is removed, because we do not really need it and the package is apparently not ready for production.
As we are now only using the table node from text there is no toolbar with markdown editing options anyway.
Edit-mode on double-lick simply works :)
Thank you for this suggestion! I have implemented and tested it with different users editing the table at the same time and it seems to work in the UI.
|
Nice catch! Fixed :) |
Thank you for the feedback, @marcoambrosini ! The editor has changed again now as it is not shipping all the markdown editing options anymore. Could you take another look at it and let me know what you think? Thank you! |
You were definitely right to question this! The implementation with the editor and all its different markdown possibilities was a bit too much and confusing for the use case of a simple table. |
8c19e8d to
0d580ea
Compare
marcoambrosini
left a comment
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.
🎉
| } | ||
|
|
||
| // Use the dedicated createTable function for table-only editing | ||
| this.editor = await window.OCA.Text.createTable({ |
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.
The table cells (first head column in this example) have hard to read contrast when selected with double-click. This only happens in the whiteboard table-edit modal, not when using the text app directly. Maybe it is missing styles from a wrapping element? It seems like the selection background is set to transparent.
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.
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.
I'm on it
|
The collaboration and edit locking is tested and works good to prevent conflicts 🎉 |
738d8b2 to
b3ac8f3
Compare
Signed-off-by: silver <[email protected]>
Implement core functionality for inserting and editing markdown tables as images in whiteboards: - Add useTableInsertion hook with double-click editing support - Add tableToImage utility for markdown-to-SVG/PNG conversion - Support inline markdown formatting (bold, italic, code, links) - Use Nextcloud Text editor integration via Vue dialogs - Preserve table markdown in customData for re-editing Signed-off-by: silver <[email protected]>
Integrate table functionality into the whiteboard interface: - Add "Insert table" menu item to Excalidraw menu with mdiTable icon - Create TableEditorDialog using Nextcloud Text editor component Requires Text app to be installed and enabled for table editing. Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Implement client-side locking to prevent concurrent table edits: - Lock stored in element.customData.tableLock with 5-minute timeout - Lock acquisition checks and shows error if table locked by another user - Lock automatically cleared when user saves/cancels edit - Lock state syncs via normal onChange flow and survives reconciliation - No heartbeat needed - timeout sufficient for cleanup Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
Signed-off-by: silver <[email protected]>
b3ac8f3 to
66893ee
Compare
|
I rebased to resolve an import conflict in App.tsx |
- Remove markdown generation from submit flow (only generated on-demand for Text editor input) - Remove tableMarkdown from customData storage across codebase - Update tests to use tableHtml instead of tableMarkdown Signed-off-by: silver <[email protected]>
66893ee to
9638756
Compare
juliusknorr
left a comment
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.
Tested on master and stable29
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |



Summary
Needs this PR in Text: nextcloud/text#8002
Adds support for inserting and editing markdown tables in whiteboards using the Nextcloud Text editor.
Features
Technical Details
customDataforeignObjectRequirements
Screenshots
To Do