Problem/Motivation
I tried to enable only the paragraph block type on a content type. When I am then editing content, this is what I expect to be the only enabled block. However, I also got the media block, which I did not want.
Steps to reproduce
Enable media module. Enable gutenberg on a content type. Disable all of the blocks. Enable paragraph block.
Also see attached animated screencast from test running.
Proposed resolution
Not enable the media block if the user does not want to.
Remaining tasks
I wrote a test for the problem. What is remaining is write a fix. Review.
User interface changes
Added a new option in the UI for the content type editing
API changes
none
Data model changes
none
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3305966-test-only.patch | 4.46 KB | eiriksm |
| gutenberg bug.gif | 1.06 MB | eiriksm |
Issue fork gutenberg-3305966
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
eiriksmPushed a merge request with a test only that I expect will fail.
I noticed another bug, that the allowed blocks were not parsed at all, so I guess the test will uncover that as well. I will push a fix for that at the same time if I have to.
Comment #4
eiriksmJust going to point out that this test now correctly proves also that limiting allowed blocks is currently broken beyond what this issue describes :)
So here is a patch with the test + the patch from #3305967: Allowed blocks not passed to editor which fixes the broken allowed blocks limiting, so it should now show the actual bug described in this issue.
Comment #6
loze commentedThis worked for me, thanks.
Comment #7
loze commentedActually I was mistaken. This does not do what i expected. With this patch all blocks are available, regardless of the setting on the node type page.
With or without this patch, I do not see a "media" option in the list of gutenberg blocks on the node type edit page. So im not able to unselect it.
Comment #8
eiriksmThat's correct. The MR currently only contains a failing test
Comment #9
eiriksmSetting to needs work since it only contains the test for now
Comment #10
eiriksmWhew, what a ride. I see I opened this a year ago. Time to close this, and to increase the test coverage on the project in the process I think.
So, there's a couple more things happening in this MR:
I wanted to add a couple FunctionalJavascript tests for this, but it seemed those were not running so well in gitlab CI because of #3403222: Move the vendor folder to a different name. So this MR also includes a workaround for that, so we can add JS enabled tests to this project.
Then to what is actually happening here:
- First, we add a test to check if the Media block is showing, even if we never actually allowed that (we only allowed the paragraph block).
- Then there is a test to check if we can enable it and it will show. Well that's certainly not possible, there is no such option. So the test is failing because of this missing option.
- Then, since it's not possible to actually allow the media block (there is no option for this) we now add this option to the form
- Then added to the media JS file, to only register the media block if this block is allowed.
- Both the tests are now passing. We can enable and disable the media block at will
However, this leaves an important change. Since it was never possible to disable the block, that means this block has been enabled on all sites that has the media module enabled (whether they want to or not). And chances are that many people actually want this. So it would be rude to now require them to go in and change things around and allow that specific block to be able to continue using their site exactly as it was before they updated. So the MR also includes an update hook to enable the block on all gutenberg enabled content types. And a test that ensures that this is in fact working.
There were much back and forth to get this finally working, but I am happy to try to get some test-only patches up here (or just link to the pipelines that are relevant)
@loze: Please give this a test and see if it helps you!
Comment #11
eiriksmComment #12
eiriksm