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

Normalize list in list; remove empty elements in list items #4583

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

tiberiuichim
Copy link
Contributor

@tiberiuichim tiberiuichim commented Mar 21, 2023

Fixes #4538

@netlify
Copy link

netlify bot commented Mar 21, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit c034141
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/641ca88fc88b8f0008a34600

@cypress
Copy link

cypress bot commented Mar 21, 2023

Passing run #4678 ↗︎

0 497 20 0 Flakiness 0

Details:

Merge branch 'master' into improve_list_in_list
Project: Volto Commit: c03414182f
Status: Passed Duration: 14:24 💡
Started: Mar 23, 2023 7:33 PM Ended: Mar 23, 2023 7:47 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@tiberiuichim
Copy link
Contributor Author

vokoscreenNG-2023-03-22_12-23-34.webm

So the solution is to automatically fix the structure to the desired state in the normalizeNode.

@tiberiuichim tiberiuichim changed the title [WIP] Render span elements; wrap text inside lists in spans [WIP] normalize list in list Mar 22, 2023
@tiberiuichim tiberiuichim changed the title [WIP] normalize list in list Normalize list in list; remove empty elements in list items Mar 22, 2023
@tiberiuichim
Copy link
Contributor Author

@davisagli Somehow the latest slate doesn't play well with cypress. See this PR, with just the lib upgrade, where a lot of tests fail: #4593

This was a solution suggested on the web somewhere.

@@ -19,7 +19,7 @@ import {
export function syncCreateTableBlock(rows) {
const id = uuid();
const block = {
'@type': 'table',
'@type': 'slateTable',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why this wasn't flagged before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ups

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but see comment.

@@ -12,6 +12,16 @@ before(function () {

beforeEach(function () {
cy.log('Setting up API fixture');

// avoid a mysterious test failure with upgrade to slate-react 0.91.4
const resizeObserverLoopErrRe = /^[^(ResizeObserver loop limit exceeded)]/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wut? T_T

"slate": "0.84.0",
"slate-hyperscript": "0.81.3",
"slate-react": "0.83.2",
"slate": "0.91.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this affect existing sites in any way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneridagh no, I don't think so.

The slate upgrade alone makes our tests fail, I have one PR with only that change. So, regardless of anything, if we want to make the upgrade, we need to do the test fix.

@tiberiuichim tiberiuichim changed the title Normalize list in list; remove empty elements in list items [DRAFT] Normalize list in list; remove empty elements in list items Mar 23, 2023
@tiberiuichim
Copy link
Contributor Author

tiberiuichim commented Mar 23, 2023

@sneridagh don't merge yet, I need to run more tests user fault, I was testing with Volto master, all good so far.

Though let's not rush with this. I'll run more tests, think about it more. If anyone is willing to test this, please do.

@tiberiuichim tiberiuichim changed the title [DRAFT] Normalize list in list; remove empty elements in list items Normalize list in list; remove empty elements in list items Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

volto-slate does not properly support "list in list" as generated by Plone Classic
2 participants