@rkoller followed up in slack
yes the focus is probably out of scope of the initial issue and could be fixed in a follow up.
Moving this one back to RTBC.
@aaronpinero , could you create the follow up issue? TIA!
@rkoller added in slack.
the background spans across the entire viewport that way the skip to main link label is legible again but there is another detail/problem in this context, the a focus outline is not visible like in gin in this case (sc2.4.7). not sure if that is what comment number 5 expresses with “exempt :focus or :focus-visible”?
Moving to Needs work.
Last commit was "WIP on testimonials.", meaning Work in Progress.
Moving back to Needs work.
Thanks for the new release. Marking as RTBC.
Tests failing, Moving back to Needs Work.
seems to ignore dependencies
Can you define what dependencies are ignored? The problem and steps to reproduce are quite vague.
This looks great. Thanks for the contribution!
Adding credit based on the MR.
Looks good to me. Over to Adam for final review. Marking as RTBC.
Moving to Needs Work for the comment in #7
Marking as Fixed. Please re-open if you need to credit additional folks.
Marking as RTBC
This looks great to me.
Added a suggestion. Moving back to Needs work also for needing tests.
Moving to Needs work for the comments in #6.
Also, note the forthcoming language and format for the API key in ✨ Clearer instructions during the AI Installer Active .
Suggestions added. Looks good, moving to RTBC.
MR added.
thejimbirch → made their first commit to this issue’s fork.
Tests are failing in CI
Makes sense to me.
Adding Cookbook link. Editing for less than 1000 characters (why????)
phenaproxima → credited thejimbirch → .
For the purpose of reapplying certain config actions following some events, we would have to find a way to identify and extract those from recipes when they initially get applied and keep them somewhere so that we can refer to them when needed.
I don't think it is necessary for ECA to identify and extract what config actions it wants to re-apply. I think the recipe should leave behind the ECA with the config actions it wants to apply on events. We should put that on the recipe author to create and maintain since we won't want all actions in a recipe to be re-applied.
Looks good, thank you for the contribution!
Marking as RTBC.
thejimbirch → created an issue.
Shouldn't this be on one of the AI modules, and not the Paragraphs module? I don't see how this could be used unless the Paragraphs module had a dependency on the AI module which I don't think would happen.
MR added.
Drupall 11 support has been added in the 3.x branch of extra_field.
composer require 'drupal/extra_field:3.0.x-dev@dev'
I think now that Drupal 11 release, the bot is no more. I have closed all of mine.
Hopefully folks on 11 are up to 11.1, but your change makes it explicit. I applied the suggestion.
thejimbirch → created an issue.
We met with @alexpott, the lead maintainer of the Recipes initiative. The long term goal of of recipes is that they are unpacked and removed from Drupal in the application process. The fact that the recipes are remaining in Drupal CMS, and this issue to forever re-apply them may lead to the recipes getting updated and havoc unleashed when reapplied.
Instead of re-applying the recipe, could there be an ECA action to apply config actions? So rather than re-applying a whole recipe, we only list the config entities and config actions we want to apply in the ECA.
Looks like a new MR has been created with less changes. Hopefully this will get us for a full Drupal 11 release. Marking as RTBC to see if @pcambra can review.
Tests pass on MR 31. Updates made by @Fabsgugu no longer give an error.
Marking as RTBC. Thanks so much!
4 Failing PHPstan tests remain.
Line composer/Plugin/Unpack/UnpackCollection.php
------ ----------------------------------------------------------------------
36 Method Drupal\Composer\Plugin\Unpack\UnpackCollection::getIterator()
return type with generic class ArrayIterator does not specify its
types: TKey, TValue
🪪 missingType.generics
------ ----------------------------------------------------------------------
------ -------------------------------------------------------------------------------------------
Line core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php
------ -------------------------------------------------------------------------------------------
Ignored error pattern #^Method
Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:cloneFixtureProjects\(\)
has no return type specified\.$# (missingType.return) in path
/build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php
was not matched in reported errors.
Ignored error pattern #^Method
Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:createIsolatedComposerCacheDir\(\)
has no return type specified\.$# (missingType.return) in path
/build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php
was not matched in reported errors.
Ignored error pattern #^Method
Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:tearDown\(\)
has no return type specified\.$# (missingType.return) in path
/build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php
was not matched in reported errors.
------ -------------------------------------------------------------------------------------------
[ERROR] Found 4 errors +1 to Pam’s content suggestion.
With the current diff from MR 34, I can't get menu links of the main menu to work.
Command:
drush dcer menu_link_content menu_name main --folder=../recipes/menus/content/
Error message:
Too many arguments to "dcer" command, expected arguments "entity_type_id" " entity_id".
If I try dce
drush dce menu_link_content main --folder=../recipes/menus/content/
I get the following error:
The "--folder" option does not exist
If I drop the folder part, I get:
Entity "menu_link_content" with ID "main" does not exist
10.4 has been released, so this can be reviewed.
wouters_f → credited thejimbirch → .
MR added that adds the dependency and corrects the project's name in the composer file.
thejimbirch → changed the visibility of the branch 3503637-the-recipe-needs to hidden.
thejimbirch → created an issue.
Responses in slack to that:
jurgenhaas
3 minutes ago
Good question. I'd say SEO exposes the issue, but even without SEO, a 404 response for unpublished nodes seems to be more logcial, and it won't disclose internal information. So, for me it could be a generic bug fix, not just SEO.
thejimbirch
2 minutes ago
I agree, I'd rather it in the recipe its in. That is a great ECA foundation.
Closed my issue as a duplicate of this. The solution looks promising.
Closing as duplicate.
Merging to dev. Thanks for the contribution!
thejimbirch → made their first commit to this issue’s fork.
Merging to dev. Thanks for the contribution!
thejimbirch → made their first commit to this issue’s fork.
I hit save too soon. Properties are what composer.json uses
The code in the MR looks good and the tests make sense.
My only concern is "extra" field. Since fields are things already in Drupal, and we have a heavy lift educating folks around recipes, could we user "properties"? I don't want to bike shed or anything, just trying to lower the confusion.
thejimbirch → created an issue.
Woo!
Adding them and making as Fixed. Thanks!
Added nkroger and making as Fixed.
I love the ECA idea. Postponing on ✨ Plugin to throw AccessDeniedException Active
Moving to RTBC based on comments in #6, thanks!
@zoocha will Do you know their Drupal handle? I can't find one with the name "Nick Kroger" Is that everyone?
Thanks. Adding credit and closing as fixed.
Merge request add that installs the
https://www.drupal.org/project/unpublished_404 →
module to the drupal_cms_authentication recipe.
thejimbirch → created an issue.
Who needs to add credit here?
Asked Catch to review in Drupal Slack since I can't assign it to him.
Strike that. Moving to Needs Review as this should have a test to validate it works. Added Needs tests tag.
Assigning to phenaproxima to review
Assigning to phenaproxima to review.
Closing as fixed and crediting the attendees. Please re-open if that was not the correct move.
Back to Needs Work for comments in #14
Postponed on the forthcoming 2.1.3 release of ECA.
There is a core issue for this.
https://www.drupal.org/project/drupal/issues/3457730 ✨ Add support for clearing the cache from the navigation menu Active
As mentioned in that issue, there is also a module:
https://www.drupal.org/project/navigation_extra_tools →
This is awesome @narendrar! I made some minor suggestions.
I assume this will have to wait until the action is in a full release of ECA, and we'll have to update all the spots that are marked dev in this MR to the new release.
thejimbirch → created an issue.