- Issue created by @flyke
- 🇺🇸United States tim bozeman
I actually just started working on integrating with navigation last Friday!
- 🇧🇪Belgium flyke
Glad to hear this. Hope you like it. I don't particulary find navigation module better than using toolbar per se, but I find using less contrib modules in favor for core modules preferable. I looked around for other alternatives for frontend editing experience but only found the discontinued quick edit module. Your video showcasing edit_plus in combination with lb_plus looks so great that it feels it should be part of drupal core or the starshot experience. If there comes a time when this works good with Navigation module, I will probably add this to all existing and new layoutbuilder projects here at work.
- Assigned to tim bozeman
- Status changed to Needs work
4 months ago 8:59pm 20 August 2024 - 🇺🇸United States tim bozeman
tim bozeman → changed the visibility of the branch 3465885-add-support-for to hidden.
- 🇧🇪Belgium flyke
Hi Tim, I know this is in development and I see that the issue branch (the diff file) is empty.
Any news on this progress ? I would love to try out edit_plus with Navigation module as your video in this issue description. - 🇺🇸United States tim bozeman
Oh sure I can push now. I mean like you said, it's a work in progress so it's pretty broken, but I'm glad for your interest! I'll push to the 3 related issues now.
- 🇺🇸United States tim bozeman
Okay I've pushed to the 3 related issues.
- ✨ Navigation module integration Needs work
- ✨ Navigation module integration Needs work
- ✨ Navigation module integration Needs work
I think you'll want to do something like the instructions on the instructions toolbar+ project page → , but with those MR's checked out.
composer config minimum-stability dev composer require drupal/lb_plus:dev-2.1.x --prefer-source composer require drupal/toolbar_plus:dev-1.0.x --prefer-source composer require drupal/edit_plus:dev-1.0.x --prefer-source composer require drupal/field_sample_value:dev-1.0.x --prefer-source drush si demo_umami -y; drush en lb_plus_ooo_mommy -y; drush uli; drush cr
I'm testing this on D11 in chrome fwiw. There are some safari issues I need to get into one day 🫣
- 🇺🇸United States tim bozeman
Oh and you'll need to apply this core patch too.
🐛 Pass attributes to create_attribute in top-bar template RTBC - 🇧🇪Belgium flyke
I added the Drupal core patch in the patches section of my composer.json file:
"#3370565": "https://git.drupalcode.org/project/drupal/-/merge_requests/4269.diff"
I added this MR as repository in my composer file:
"repositories": [ { "type": "git", "url": "https://git.drupalcode.org/issue/edit_plus-3465885.git" },
I added this issue version of edit plus:
composer require drupal/edit_plus:"dev-3465885-navigation as 1.0.x-dev" -W
And then I enabled edit_plus:
drush en edit_plus
It is requiring me to enable the toolbar and toolbar_plus modules at this moment. I think that should not be necessary if you are using navigation module ?
- 🇧🇪Belgium flyke
Anyway, I selected yes to install toolbar, toolbar plus and edit plus, all seemed good.
> [notice] Standaardconfiguratie bijgewerkt naar nl > [notice] The configuration was successfully updated. 608 configuration objects updated. > [notice] Message: De configuratie is succesvol bijgewerkt. Er zijn /608/ configuratieobjecten > bijgewerkt.
Then I cleared my caches and then I visited the website.
But I get this error:
Unknown named parameter $weight
Via watchdog:
Error: Unknown named parameter $weight in ReflectionAttribute->newInstance() (regel 18 van /var/www/html/web/modules/contrib/edit_plus/src/Plugin/Tool/EditPlus.php).
- 🇧🇪Belgium flyke
I was finally able to narrow down the cause of the out of memory to the loading of the toolbar_plus/toolbar_plus library (inside
web/modules/contrib/edit_plus/edit_plus.libraries.yml
)If I comment that line, then site works, if I re-enable that line, then site goes into out of memory issues.
- 🇺🇸United States tim bozeman
Hmm, I just set this up on a new computer and it went fine. In short, I did something like:
- Start a new D11 site with ddev
composer config minimum-stability dev composer require drupal/lb_plus:dev-2.1.x --prefer-source composer require drupal/toolbar_plus:dev-1.0.x --prefer-source composer require drupal/edit_plus:dev-1.0.x --prefer-source composer require drupal/field_sample_value:dev-1.0.x --prefer-source composer require drupal/twig_events:dev-1.0.x --prefer-source
- Goto each of the 3 MR's and click the show commands link then fetch > checkout MR branch
- Manually apply the 1 line core patch
- Run
drush si demo_umami -y; drush en lb_plus_ooo_mommy -y; drush uli; drush cr
I don't expect any of this to work outside of that context atm.
- 🇧🇪Belgium flyke
Hi, thanks for the feedback.
I fully understand babysteps and everything only working under the exact versions and under a fresh site install.Unfortunatly I am a bit stubborn and try to get this working on our company's starterkit project that will serve as a base for new projects.
Because thats ultimatly where it will be needed, so its good to test conflicts with other modules and custom ones that we will have.
So this comment is more for me as documentation I guess, some problems might be specific to this project because of its modules and settings.Problem 1
After installation, you get this error:
Unknown named parameter $weight
Solution
Editedit_plus/src/Plugin/Tool/EditPlus.php
and remove line 21:weight: 20,
Problem 2
Page won't open, keeps loading until out of memory error
Solution
There is a circular reference:edit_plus/edit_plus.libraries.yml
requires the toolbar_plus/toolbar_plus library, andtoolbar_plus/toolbar_plus.libraries.yml
requires the edit_plus/library library. Solution is to edit toolbar_plus.libraries.yml and remove or comment:- edit_plus/library
Problem 3
Not finding or seeing any link or button for edit_plus on the page. Expected this to be in the navigation_top_bar or in the navigation sidebar but cant find it. Todo: look into it.Problem 4
Lots of undefined array key erros. Todo: look further into it.Warning: Undefined array key "#configuration" in block_theme_suggestions_block() (line 202 of core/modules/block/block.module). Warning: Trying to access array offset on null in block_theme_suggestions_block() (line 202 of core/modules/block/block.module). Warning: Undefined array key "#plugin_id" in block_theme_suggestions_block() (line 214 of core/modules/block/block.module). Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in block_theme_suggestions_block() (line 214 of core/modules/block/block.module). Warning: Undefined array key "content" in block_content_theme_suggestions_block_alter() (line 195 of core/modules/block_content/block_content.module).
- 🇺🇸United States tim bozeman
Great! I’m glad you got it going. I haven’t added the save button back to this new UI yet 😅
- 🇧🇪Belgium flyke
Maybe the module should have a settings page where you can choose to have 'instant save', so that each time you adjust something, the page is immediatly saved and changes are immediatly visible.
Anyway, thank you for the work you are doing, this really is going to be a great content editor experience when its done. - 🇺🇸United States tim bozeman
You are very very welcome! We are so excited!
Auto save, hmm interesting idea.
So far we've mirrored layout builder where the editing and layout changes are automatically saved to a tempstore, but yeah it does require clicking the save button to "commit" the changes. Layout Builder, Edit+, and our upcoming tools need to use the same temporary storage since the changes should all show up. The idea behind the tempstore is that if we just saved after every change we'd balloon the revision table when using workspaces (and maybe without workspaces? idk). Andrei had a really neat idea recently that layout builder and edit plus should use forward revisions instead of a tempstore which would be neat. I think auto-saving might be a little easier then, though I'm sure it's possible now too 🤔
- 🇧🇪Belgium flyke
Should autosave feature be taken up with layoutbuilder? So that maybe its part of the whole tempstore code that layoutbuilder also uses and that layout tempstore autosave is a more global (core) feature instead of just for this module ?
Anyway, its a nice to have, not a must. I only thought about it because I saw no save button yet in the current version of edit_plus that I'm testing so the whole edit_plus was kinda useless. But if/when the save button is fixed (in navigation top bar?) then the need for autosave is also gone. Would still be a nice to have. - 🇧🇪Belgium flyke
btw, after a lot of trial and error, I was finally able to update my project to D11 (11.0.1).
Now when I open a page I get this error:
Drupal\edit_plus_lb\EditPlusLbTempstoreRepository::get(): Argument #1 ($entity) must be of type Drupal\Core\Entity\EntityInterface, null given, called in /var/www/html/web/modules/contrib/edit_plus/src/ParamConverter/EntityConverter.php on line 39
That line in question is:
$tempstore_entity = $this->editPlusTempstoreRepository->get($entity);
- 🇧🇪Belgium flyke
Before enabling toolbar_plus, edit_plus and edit_plus_lb there are no errors on my page.
After enabling them, there are errors:User warning: The following theme is missing from the file system: toolbar_plus in Drupal\Core\Extension\ExtensionPathResolver->getPathname() (line 63 of core/lib/Drupal/Core/Extension/ExtensionPathResolver.php). Deprecated function: dirname(): Passing null to parameter #1 ($path) of type string is deprecated in Drupal\Core\Extension\ExtensionPathResolver->getPath() (line 85 of core/lib/Drupal/Core/Extension/ExtensionPathResolver.php). User warning: The following theme is missing from the file system: toolbar_plus in Drupal\Core\Extension\ExtensionPathResolver->getPathname() (line 63 of core/lib/Drupal/Core/Extension/ExtensionPathResolver.php). Deprecated function: dirname(): Passing null to parameter #1 ($path) of type string is deprecated in Drupal\Core\Extension\ExtensionPathResolver->getPath() (line 85 of core/lib/Drupal/Core/Extension/ExtensionPathResolver.php). Warning: Undefined array key "content" in block_content_theme_suggestions_block_alter() (line 195 of core/modules/block_content/block_content.module). Warning: Undefined array key "#configuration" in template_preprocess_block() (line 254 of core/modules/block/block.module). Warning: Undefined array key "#plugin_id" in template_preprocess_block() (line 255 of core/modules/block/block.module). Warning: Undefined array key "#base_plugin_id" in template_preprocess_block() (line 256 of core/modules/block/block.module). Warning: Undefined array key "#derivative_plugin_id" in template_preprocess_block() (line 257 of core/modules/block/block.module). Warning: Undefined array key "content" in template_preprocess_block() (line 260 of core/modules/block/block.module). Warning: Trying to access array offset on null in language_preprocess_block() (line 350 of core/modules/language/language.module). Warning: Trying to access array offset on null in menu_ui_preprocess_block() (line 441 of core/modules/menu_ui/menu_ui.module). Warning: Trying to access array offset on null in node_preprocess_block() (line 456 of core/modules/node/node.module). Warning: Trying to access array offset on null in user_preprocess_block() (line 341 of core/modules/user/user.module).
- 🇺🇸United States tim bozeman
Interesting. I guess I’m going to be in the same boat. I’m supposed to be getting all this going on a real site. At least enough for a demo, in the next 2 weeks.
It’s a real complex site too 😅
- 🇧🇪Belgium flyke
That is great to hear because that means you could encounter real-world problems of using it and are required to fix them.
Making this better for everyone.
For our projects we will not be using edit_plus for now because of multiple issues at the moment (the lack of the possiblity to save, the off canvas layout when bootstrap_layout_builder is enabled, different errors when trying to edit different block types, ..)
But I would be glad when a time comes that all is stable and usable.For now I am glad to be using lb_plus module, in combination with our own custom blocks and block icons, and also in combination with bootstrap_layout_builder and bootstrap_styles, and some custom code that fixes some issues I am having in lb_plus module.
Attached video is how it all looks together.
- 🇺🇸United States tim bozeman
Wow that looks great!
I am running into #22 now. Thanks for your research on that one!
- 🇺🇸United States tim bozeman
For #22 ✨ Navigation module integration Needs work it's really just that the navigation module's config assumes you have shortcuts installed. I just removed this part from their config and reimported and those errors went away.
- 🇺🇸United States tim bozeman
🐛 Pass attributes to create_attribute in top-bar template RTBC This was the core patch I was talking about btw.
-
tim bozeman →
committed 66476cea on 1.0.x
Issue #3465885 by tim bozeman, flyke: Navigation module integration
-
tim bozeman →
committed 66476cea on 1.0.x
- 🇺🇸United States tim bozeman
I think this is probably a good enough stopping point. Let's throw this against the wall and see what breaks.
- 🇧🇪Belgium flyke
Great to see that you have reached a point where there is a release for testing !
I cant test it for now because with our custom modules and theming the fact that this requires toolbar module gave me some problems and headaches that I did not have the time for to resolve.So in order to still have some frontend editing, I went from using these modules:
- lb_plus
- edit_plus
- toolbar
- toolbar_plusto using:
- layout_builder_ipe
- a custom module that moves the layout_builder_ipe button into the navigation top bar. It also styles the layout edit page so it looks more like how it looks in lb_plus.I still think edit_plus looks far more superior from what I can see in your video, so I will eventually get back to this.
- 🇺🇸United States tim bozeman
Okay cool.
FWIW, you shouldn’t need cores toolbar module anymore once you turn on the navigation module. This latest version you don’t need to go to the layout tab anymore. It’s just switching tools kinda like photoshop. The next two big things are going to be breaking out all the LB context menu things like remove block, duplicate, block and all the other stuff like moving blocks and sections etc into tools too. Kinda removing the layout tool all together. Then after that we’re going to make a styling tool.
- 🇧🇪Belgium flyke
Thanks for the feedback.
I see now in the composer.json and toolbar_plus.info.yml files of toolbar_plus that there is indeed no requirement for the toolbar module, which I believe were there before.
I probably get this wrong, but I understand that in the future for edit_plus if you want to remove a block from the layout, you cant instinctively find that on the block you want to remove itself ? You need to first click on a remove tool that is not located on or near the block you want to remove, and only after that can you go to the block you want to remove and then remove it ?
A hover -> delete button or permanently visible delete button like in attached demo seems more intuitive to me. But that is just speculating of course, I will test and review edit_plus when all stuf like moving, deleting, changing config etc has been worked out. I still believe that Drupal could do with an improved in place editing experience, and I'm very glad to see that you are going out of your way to give this a shot.
- 🇺🇸United States tim bozeman
Okay cool. I think I conflated those two things though. Edit+ is just the inline editor and aside from continuous improvement, is mostly feature complete at this point. I do want to do a bunch of performance enhancements though.
That other stuff I was talking about will be in lb_plus where we’re still reimagining that UI. I think we are pausing on that to let the dust settle for a while, but just wanted to share the road map.
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇧🇪Belgium flyke
Hi,
I think the video in the description of this issue is misleading, since it's also showing adjusting layout etc I think.
But I am starting to partially get it, probably still wrong about some stuff here.
edit_plus / edit_plus_lb: in-place editing of plain texts and wysiwyg texts. Works on my testing on some of our custom layout builder blocks but not on all of our custom layout builder blocks that have multiple fields including a plain or long text field.
lb_plus / lb_plus_edit_plus: for editing sections, blocks, images, ... Could not get this properly working on our test site. - 🇺🇸United States tim bozeman
Ah sorry about that. In the video I was mainly just trying to show the new idea of "Tools" and how you can quickly switch between editing the fields and messing with the layouts by clicking the tools kinda like photoshop.