🇩🇪Germany @jurgenhaas

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is now fully implemented and tested. The code feels cleaner now, so I'm pretty excited about that opportunity.

Note, that the hook_module_implements_alter can only be implemented the old way, and we had that for eca_form and eca_project_browser to change the order of hook execution. I've removed those hooks entirely, and managed the processing order with an attribute.

Will merge this, when tests are all green, as I don't expect anyone to review this. The changes are massive and hard to compare, as stuff went into different files, so the diff shows big junks of deleted and new code, which doesn't relate to each other.

Another note: auto-wiring was a bit of a challenge, but using aliases in service files resolved it eventually.

And last but not least, I've also disabled manual hook scans by a parameter in the services file.

jurgenhaas → changed the visibility of the branch 2.1.x to hidden.

jurgenhaas → changed the visibility of the branch 3.0.x to hidden.

As mentioned before, this will go into 3.x and we don't require BC for it. Also, as the issue fork is a few months old and also has BC in it, etc. I've decided to create a new branch and start from scratch.

And yes, I agree with @mxh in #6 that this will also make a few existing classes redundant, as we can re-use them for this purpose.

This has now been completed. The storage options can be configured in the Modeler API, and the Model config entity in ECA got deprecated and will be removed in version 4.0.0.

The ECA UI submodule provides a couple of services, to make the upgrade seamless:

• There is an update hook that automatically migrates all existing models to the new format
• There is an event subscriber that migrates models when they get imported, of course only when required
• There is a drush command eca:v3:migrate which checks all existing models and migrates them if necessary

The migration of a model is not dangerous, as it keeps the ECA config entity as is, and only adds third-party settings with details that otherwise had been stored in the eca.model.[ID] config entity. The latter gets deleted afterwards.

@hchonov any chance to continue working on this?

Great finding, thanks @abhisekmazumdar

I've only changed the sorting of the use-statements as well, and merged this into 2.1.x and 3.0.x

jurgenhaas → made their first commit to this issue’s fork.

Time to mark this as fixed as well. All child issues have been completed and the related issue ✨ ECA Render: translate all renderable strings Active is no longer postponed and can be addressed as a feature request.

Now that 🌱 [META] Implement complete coverage of validatable config schema across plugins and entities Active and all related issues have been completed, we can get back to this one.

I wonder if config translation wouldn't already provide everything we need. Let's give that a try.

Yeah, all tests are green. What an achievement. Thank you so much @michaellander, @lammensj, and everybody else who made this possible.

This is now mostly done, only 4 tests are still failing like with this message:

1) Drupal\Tests\eca_workflow\Kernel\WorkflowTransitionTest::testExecuteWithNoEntity
Drupal\Component\Plugin\Exception\PluginNotFoundException: The "eca_workflow_transition:editorial" plugin does not exist. Valid plugin IDs for Drupal\Core\Action\ActionManager are: node_make_unsticky_action, node_make_sticky_action, node_promote_action, node_unpromote_action, user_unblock_user_action, user_remove_role_action, user_cancel_user_action, user_block_user_action, user_add_role_action, entity:unpublish_action:node, entity:save_action:node, entity:save_action:user, entity:publish_action:node, action_message_action, action_goto_action, action_send_email_action, entity:delete_action:node
/builds/project/eca/web/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:53
/builds/project/eca/web/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryCachedTrait.php:28
/builds/project/eca/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:16
/builds/project/eca/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:85
/builds/project/eca/src/PluginManager/Action.php:178
/builds/project/eca/modules/workflow/tests/src/Kernel/WorkflowTransitionTest.php:101

I'löl fix that over the weekend, and then we should be able to finish this off. What a big one !!!

Just realized that we have to add schema entries for all plugins even if they have no config. This is because ECA always adds the configuration property, in these cases with an empty array. I've added them for migrate and also for all the other missing ones. Found out because tests were failing ;-)

Confirmed by @catch on Slack.

Fixed and published 5.0.2

xjm → credited jurgenhaas → .

Sorry for that, this is actually a Gin thing, not Gin Toolbar. Moved it and will fix right away.

Note: The opacity and blur, that I described a "Problem 2" in #3 is by design.

Gin 4 (and Gin Toolbar 2) do not support Drupal versions below 11.2 any longer.

You need to update to Gin 5 and Gin Toolbar 3. You can use this command:

composer require drupal/gin:^5 drupal/gin_toolbar:^3

@catch this has now been resolved in Gin and Gin Toolbar. Patch releases will follow later today.

Here in Drupal CMS, there doesn't seem to be any todo, so it can possibly be closed.

Got a good to go on Slack.

This is awesome, thanks @saschaeggi for fixing this.

Turns out, we can make lives even easier. With ✨ Remove dependency from toolbar module Active and 🐛 Gin makes Navigation toolbar position fixed Active it's possible to make the toolbar module completely optional, and it requires only minor changes.

As a result, if neither navigation nor toolbar is enabled, Gin and Gin Toolbar still work properly, there's just no toolbar, but that's fine.

Waiting for a review in Gin Toolbar, the change in Gin has already been pushed to the dev release as it is a safeguard that should have been there already before.

Turns out, the solution is even simpler than that: there are only 2 places where the code depends on the toolbar module, and they can be called conditionally and all should be fine. That way, it's down to the user, if they want to enable the toolbar module or not.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is fixed in the dev release. A patch release 5.0.1 will be published later today.

There are some explicit decisions in Gin's top-bar.css where I'm not aware why:

Problem 1: top position of the main canvas

The navigation module sets that to the height of the top bar:

  .top-bar:has(.top-bar__tools:not(:empty), .top-bar__context:not(:empty), .top-bar__actions:not(:empty)) ~ .dialog-off-canvas-main-canvas {
    margin-block-start: var(--admin-toolbar-top-bar-height);
  }
}

Gin sets the margin-block-start: 0 which is because Gin also sets the height of the top bar to auto, so it doesn't know the height.

Problem 2: top bar opacity

Gin sets opacity and a blur on the top bar.

We need to find out why that has been don that way before we make any changes. @saschaeggi could you please throw some light on this?

benjifisher → credited jurgenhaas → .

Thank you @kmonty for commenting. We usually do exactly that, but this time it wasn't possible. The constraint on Drupal core was defined as ^11 which would allo version 4.0 to be installed on Drupal 11.2, but it's not compatible with it. Therefore, we had to create 4.1 which makes it clear that it only works with Drupal below 11.2, and 4.0 to be unsupported. There is no difference between the latest 4.0.6 release and 4.1.0, so updating comes with no risk.

jurgenhaas → created an issue.

Please use the dev releases of modeler_api and bpmn_ui for testing, things have changed and continue to do so permanently.

Had to open a follow-up issue for this at 🐛 List compare action fails with complex data Active .

jurgenhaas → created an issue.

Ah thanks for updating, although I think the first sentence is also reversed there still?

You're right, now corrected as well. Looks like my caffeine level is not sufficient yet.

I think this could be handled...

I need to take some time testing this. We would then have to automatically enable/disable toolbar when navigation gets disabled/enabled.

Won't be able to look into that before next week, though.

Sorry, I mistyped the second case in #4 and just corrected it.

So, here are the scenarios:

• Navigation enabled: Gin Toolbar may not require toolbar, but that needs to be verified. At least it suppresses its display.
• Navigation disabled: Gin Toolbar requires toolbar and displays like before

installed the site a day or two ago

We've fixed that in Gin and Gin Toolbar only a day or two ago, so maybe that was just an unfortunate overlap.

Could gin_toolbar remove the toolbar dependency though?

This may be possible for cases where navigation is not enabled. But if navigation is enabled, Gin and Gin Toolbar continue to behave like before Drupal 11.2. This is a bit of a nightmare, I know. Sounds like we only get rid of this when toolbar got deprecated and removed.

We still need gin_toolbar, and as a dependency also the toolbar module. The reason being that with gin_toolbar we can inject Gin styling into the frontend, something that the admin theme can't do on its own.

Version 3 of gin_toolbar disables its own display which comes from a hook. Maybe your temporary issue was caused by a missing cache rebuild?

@joachim it is mentioned in the IS under "Options" with static site generators and markdown.

We're pretty close now. There are a few missing schema errors left when PhpUnit is trying to go through all our test cases. But I have merged all other related issues from 🌱 [META] Implement complete coverage of validatable config schema across plugins and entities Active so that we can finally go through all of these remaining issues.

@michaellander thank you so much for everything you've done here together with your team. This has been a massive effort.

jurgenhaas → made their first commit to this issue’s fork.

jurgenhaas → made their first commit to this issue’s fork.