Skip to content
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

Added table tags to dashed to aid in table editing #5735

Closed
wants to merge 1 commit into from

Conversation

bernesto
Copy link
Contributor

@bernesto bernesto commented Mar 9, 2024

RTEs often include table editing features inside of [data-gjs-highlightable] nodes. Invisible elements should always have a visual indicator available to aid in editing.

May want to add some of these invisible elements to dashed as well? div, blockquote, pre, address, figure, figcaption, form, details, summary, menu, dir, section, article, aside, nav, main, details

RTEs often include table editing features inside of [data-gjs-highlightable] nodes. Invisible elements should always have a visual indicator available to aid in editing.

May want to add some of these invisible elements to dashed as well? div, blockquote, pre, address, figure, figcaption, form, details, summary, menu, dir, section, article, aside, nav, main, details
@artf
Copy link
Member

artf commented Mar 12, 2024

I'd say let's leave this decision (configurable or not) to who is creating the custom RTE plugin.
This is not something everybody would want to have and could also be considered a "visual break change".

ps. you can always extend that CSS with canvasCss option if you need for your use case.

@artf artf closed this Mar 12, 2024
@bernesto
Copy link
Contributor Author

Yes, this is an easy change to incorporate into an RTE plugin. Thanks for pointing out the canvasCss. Good point on the visual breaking change, someone who has implemented this already may have a conflict.

Maybe we should move this to the CKeditor plugin project. CKeditor in it's native mode does provide dashed outlines of table cells, but visibility would not be controlled by the GrapesJS dashed outline command, which I think would be desirable, regardless of whether a CKEditor is active or not.

Note, I am evaluating rewriting that plugin right now, as I have a lot of history with CKEditor. Specifically I'm looking at better options for managing the creation and destruction of editors, as right now, shared spaces creates a new editor for every element you edit, which pollutes the DOM, and starts to leak memory when making lots of edits.

@artf
Copy link
Member

artf commented Mar 14, 2024

Maybe we should move this to the CKeditor plugin project. CKeditor in it's native mode does provide dashed outlines of table cells, but visibility would not be controlled by the GrapesJS dashed outline command, which I think would be desirable, regardless of whether a CKEditor is active or not.

Yeah right, adding something like this to the CKEditor plugin makes more sense.
From the configuration point of view, I'd add a new option to the plugin that replaces their outlines with those from GrapesJS (eg. by using your changes in this PR).

shared spaces creates a new editor for every element you edit, which pollutes the DOM, and starts to leak memory when making lots of edits.

Yeah, I know 😞

@bernesto
Copy link
Contributor Author

Yeah, I remember Frederico talking about how there wasn't much interest in shared spaces(not sure why smh), and how it was sort of a half-assed implementation. Effectively creating lots of editors and using smoke and mirrors to show the right one... This was about the time that they started working on CKEditor 5, and I think that feature never really got touched again in v4.

Then CkEditor 5 was supposed to be more of a block editor like Quil vs. a full HTML editor like v4 was. That pretty much precluded us from adopting it. Now, I am not sure when they pivoted back to full HTML WYSIWYG (because I stopped paying attention). But, now CKEditor 5 is sic, and works perfect... but the license is a non-starter for me.

I have considered TinyMCE as well, since it is maintained and has a GPL, but unfortunately is suffers from issue of editor creep. Plus doesn't have support for mobile inline editing yet (not sure why, seems almost there... Maybe I should contribute lol, I need a clone).

All of that is to say, CK Editor V4 is still our preferred editor till Tiny is a bit more mobile friendly. And, in adopting GrapesJS, we need to get that editor as performant, featured, and reliable as possible. Good times! So, I'll contribute back anything worthwhile to that repo.

@artf
Copy link
Member

artf commented Mar 16, 2024

I was interested in integrating tiptap (based on ProseMirror), did you have any chance to check it out?

@bernesto
Copy link
Contributor Author

bernesto commented Mar 16, 2024

Yes, I have. In general, I like the concept. Headless, your own UI, etc.

I do have a little cause for pause with them though. The pro-extension / pro-account portion screams "we really want to productize this" vs. "this is something we needed anyways and just wanted to share". Intentions matter. (Note, I am not against people getting paid for their work. Just be transparent about it.) Think CKEditor 4 vs. 5. That future license may be cost prohibitive, and engineering an alternative may be costly.

The depth of integration here could be quite deep, considering you're building the entire UI.

Additionally, it just seems "pseudo-open source", where by they stand on the source shoulders of ProseMirror, and grasp at money with their business plan and collaborative editing.

<rant>It seems to be a trend lately where your whole app is spread across a bunch of service providers (auth $, db $, file $, cdn $, etc.) and merged at the client SPA... Yuck. Learn to server code. Great for a VC prototypes, not for a viable business.</rant>

Anyway, to be fair, I haven't spent a lot of time with TIpTap after the initial turn off.

I do how ever like ProseMirror "for GrapesJS". I didn't care it as an HTML editor for our internal purposes, because we work with 'all' html. That said, I think ProseMirror's intended 'constrained' approach to HTML is actually a really good fit for GrapesJS, as it would 'only' be used for Text components, and all other HTML is handled by GrapesJS. Thereby, the two of them in tandem would be a complete solution, and a good match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants