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

Experimental feature flag for new add block button enabled by default. #4947

Merged
merged 5 commits into from
Nov 5, 2023

Conversation

sneridagh
Copy link
Member

How we did we would do this?

  • Next major (17) we flip the experimental flag to true
  • In two majors (18) we make it default? (I hope that by then we have Quanta Toolbar around, tbh).

@sneridagh sneridagh requested a review from davisagli July 5, 2023 11:09
@netlify
Copy link

netlify bot commented Jul 5, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 0fbdd7c
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/654781214d9a59000891d21c

@cypress
Copy link

cypress bot commented Jul 5, 2023

4 failed tests on run #7051 ↗︎

4 557 20 0 Flakiness 0

Details:

Merge branch 'master' into newbuttonoutofexperimental
Project: Volto Commit: 6566e3d2fe
Status: Failed Duration: 17:36 💡
Started: Aug 29, 2023 10:47 AM Ended: Aug 29, 2023 11:05 AM
Failed  blocks/block-anchors.js • 1 failed test • Core Blocks 18.x

View Output Video

Test Artifacts
Block Tests: Anchors > Add Block: add content to TOC Output Screenshots Video
Failed  volto-slate/03-block-slate.js • 1 failed test • Core Volto Slate 18.x

View Output Video

Test Artifacts
Block Tests > should show block chooser btn on adding new text block created from the previous block with the formatted content Output Screenshots Video
Failed  volto-slate/03-block-slate.js • 1 failed test • Core Volto Slate 16.x

View Output Video

Test Artifacts
Block Tests > should show block chooser btn on adding new text block created from the previous block with the formatted content Output Screenshots Video
Failed  blocks/block-anchors.js • 1 failed test • Core Blocks 16.x

View Output Video

Test Artifacts
Block Tests: Anchors > Add Block: add content to TOC Output Screenshots Video

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

@rotavio
Copy link

rotavio commented Jul 5, 2023

We are using some addons that allows adding blocks inside other blocks, like columns, tabs and accordion. There are some issues with these blocks when the experimental feature is enabled.

Is the "block inside block" feature native or created by the addons team?

Is there something like this native to Volto? If no, are there plans for it?

@sneridagh
Copy link
Member Author

@rotavio there is no such add-ons team, only their authors. These change the behavior of the add button in the main stream of blocks. The management of the "blocks in block" use case is done internally by the add-ons (using their own handlers).

Recently we included in latest 17a16 the primitive container and the grid block in core. However, these are also detached from the mentioned feature, having their own means and UX to add blocks inside.

I am personally not aware that there are issues with other addons (there are none in the core grid).

/cc @avoinea @tiberiuichim ?

@rotavio
Copy link

rotavio commented Jul 5, 2023

Thank you for the information. I understand.
What I mean by issues are not exactly errors, but little annoyances when adding blocks. You have to add a text block. The add button appears only on the last text block of the array.
But i understand how this is not a volto issue. Thanks!

@sneridagh
Copy link
Member Author

@rotavio It would be great if you could provide exact instructions for reproducing the annoyance, and screenshots/video. Which add-on are you working with? etc. Thanks!

@sneridagh sneridagh added this to the Plone 6.1 milestone Jul 18, 2023
@sneridagh
Copy link
Member Author

@plone/volto-team I had further discussions regarding this matter with a couple Team members who brought up good points:

1.- We really haven't yet a proper alpha testing for including the feature by default

This is utterly true, and I didn't realise this has to be the case always. So doing it now at the end of the alpha phase of 17 is useless.

2.- The feedback is still quite few, and a proper alpha phase would have helped. Mostly because of 1.

3.- We still are unsure of the final shape that the Experimental / Future / Feature flags should have, we decided not to change the current shape

So I propose to slate this until 18, the first alpha would have it enabled by default (in the shape of the current experimental flag, but turned on). During beta/RC/final release, and if we decide to have it as a default feature, then it should abandon the form of the "experimental" flag and be just another "settings" key (turnable off).

@sneridagh sneridagh modified the milestones: Plone 6.1, 17.x.x, 18.x.x Sep 15, 2023
@sneridagh sneridagh merged commit 255740e into main Nov 5, 2023
48 checks passed
@sneridagh sneridagh deleted the newbuttonoutofexperimental branch November 5, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants