search

plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
426 stars 576 forks source link

Slate integration #3313

Closed tiberiuichim closed 1 year ago

netlify[bot] commented 1 year ago

Deploy Preview for volto canceled.

Name Link
Latest commit 3b149d91fab6a7c2498c0b5d2ab7cd437d238189
Latest deploy log https://app.netlify.com/sites/volto/deploys/62d80fb1c9eb450008a23245
tiberiuichim commented 1 year ago

Listing here things that I've ripped out and will have to be added back:

cypress[bot] commented 1 year ago


Test summary

349 • 0 • 15 • 0

Run details
Project Volto
Status Passed
Commit 3b149d91fa
Started Jul 20, 2022 2:26 PM
Ended Jul 20, 2022 2:39 PM
Duration 12:20 💡
OS Linux Ubuntu - 20.04
Browser Chrome 103

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

sneridagh commented 1 year ago

Can we name it @plone/volto-slate? 😄 Is it much trouble? It comes to my mind a couple of situations that would come in handy and avoid confusion.

sneridagh commented 1 year ago

A summary:

giuliaghisini commented 1 year ago

instead of including and repeat code in each Edit component for blocks.. is it possible to introduce a widget to simplify imports and avoiding some eventually distributed bugs?

tiberiuichim commented 1 year ago

@giuliaghisini Is this what you're thinking? https://github.com/plone/volto/blob/slate-integration/packages/volto-slate/src/widgets/RichTextWidget.jsx

giuliaghisini commented 1 year ago

@giuliaghisini Is this what you're thinking? https://github.com/plone/volto/blob/slate-integration/packages/volto-slate/src/widgets/RichTextWidget.jsx

yes! so.. why we don't use it in existing blocks instead including slate directly?

tiberiuichim commented 1 year ago

@giuliaghisini Title/Description blocks have a different behavior (need to render as a specific tag, only one line, etc). The Table also has specific requirements (navigation keys, etc).

tiberiuichim commented 1 year ago

@nileshgulia1 No, @plone/volto-slate should be provided as an alias by Volto. Maybe Volto needs to include the packages folder, when installed? Something is not working properly, Volto includes @plone/volto-slate.

nileshgulia1 commented 1 year ago

@tiberiuichim I don't have a strong opinion if we should ship packages/ folder with volto. We have the @plone/scripts package which is installed as a dependency in Volto, but not the case here with @plone/volto-slate. I think we should make a decision on it soon.

tiberiuichim commented 1 year ago

@nileshgulia1 That decision was made at the Bethoven sprint, together with @sneridagh and @robgietema. The existing layout of the packages in this PR will be kept, @plone/volto-slate will be included with Volto, but it won't be published as a separate package.

sneridagh commented 1 year ago

@tiberiuichim @razvanMiu @nileshgulia1 I assume this is ready? I realised that there are still two commented Cypress tests.

@plone/volto-team if anyone else could take a look, it would be great! More eyes are always welcome.

We are almost there!

sneridagh commented 1 year ago

mmm it seems the generator tests have problems.

sneridagh commented 1 year ago

Now, it should be green :)

sneridagh commented 1 year ago

Ok, so I've found an undesired situation in the DefaultView thanks to the new tests :)

The content is appearing for a fraction of a second with the new "block-less" view before changing to the blocks-enabled one. This fixes the tests failing: https://github.com/plone/volto/issues/3481

sneridagh commented 1 year ago

💚

tiberiuichim commented 1 year ago

green? OMG :)

ksuess commented 1 year ago

Sorry for interrupting. I think the inline StyleMenu is a funky thing that got lost and would be welcomed back not only by me. Any opinions?

does this illustrate the worth of inline styling?

image

nileshgulia1 commented 1 year ago

@sneridagh @ksuess The commit https://github.com/plone/volto/pull/3313/commits/3b149d91fab6a7c2498c0b5d2ab7cd437d238189 alters volto-slate to add a slate text block from blocksChooser. Right now if you add a new text block from BlockChooser it adds as a draftjs one.

ksuess commented 1 year ago

@sneridagh @ksuess The commit 3b149d9 alters volto-slate to add a slate text block from blocksChooser. Right now if you add a new text block from BlockChooser it adds as a draftjs one.

I think Volto 16 should do the same, that I do in a project:

config.js:

config.settings.defaultBlockType = 'slate';

sneridagh commented 1 year ago

yeah @nileshgulia1 @ksuess I wonder how cypress tests are working then? It should be ok.

could you please take care and test it? Thanks!

sneridagh commented 1 year ago

@tiberiuichim @razvanMiu Uh, sure, I'm idiot, if you need this, then you do that in your add-on/project, then it will be overrided.

razvanMiu commented 1 year ago

@sneridagh @ksuess The commit 3b149d9 alters volto-slate to add a slate text block from blocksChooser. Right now if you add a new text block from BlockChooser it adds as a draftjs one.

@sneridagh I've made this change in here only for testing something and I forgot to revert it. If someone wants to use draft js then in an addon this settings needs to be added:

config.settings.defaultBlockType = 'text'
config.blocks.blocksConfig.text.restricted = false
config.blocks.blocksConfig.table.restricted = false
config.blocks.blocksConfig.slate.restricted = true
config.blocks.blocksConfig.slateTable = true

Only the title & description blocks would still use slate. The ToC is compatible with draftjs.

ksuess commented 1 year ago

Sorry for interrupting. I think the inline StyleMenu is a funky thing that got lost and would be welcomed back not only by me. Any opinions?

I created a new issue for bringing back the StyleMenu to not block this PR.

tiberiuichim commented 1 year ago

@ksuess @sneridagh I'm in favour of including the Style menu. The only change I would do, is to use a button as a trigger, so it would take less space in the toolbar

sneridagh commented 1 year ago

Please excuse my ignorance, the StyleMenu is the one on the toolbar that is a dropdown with the name of the available styles? I'm ok to add it in the codebase, but I'd leave it as opt-in. What do you think?

sneridagh commented 1 year ago

I think that people would modify the toolbar anyways per project, depending in they need some of the buttons or not.

ksuess commented 1 year ago

Yes, sure the StyleMenu should be optional. I could imagine: if a project does https://github.com/eea/volto-slate/blob/2e77a2bec56201a5ff9791e6aad9e8fa95e8d3ab/src/editor/plugins/StyleMenu/index.js#L15-L26 then a button appears to open a dropdown. With @tiberiuichim, this would take as small space as possible and would look still slim. Will do a proposal in https://github.com/plone/volto/issues/3494 if you all aggree to bring it back. If not, I am completely happy to do it in an add-on.

tiberiuichim commented 1 year ago

@ksuess @sneridagh My vote is for the Style Menu to come with Volto, disabled.

The style menu covers 2 potential holes for a project:

sneridagh commented 1 year ago

How do we proceed? Do we merge now, then address it in another PR?

ksuess commented 1 year ago

How do we proceed? Do we merge now, then address it in another PR?

I created a new issue for bringing back the StyleMenu. This PR should not be blocked by this side topic.