🇨🇴Colombia @jidrone

There is a .DS_Store file in the patch, it needs to be removed.

jidrone → created an issue.

jidrone → created an issue.

Hi everyone,

I have been thinking on this from the DrupalCon Portland after the conversation with @ckrina and lauriii, this is my proposal:

• Create a module called Icon Library:
• Detects svg sprites or single icons from any module or theme.
• Provides field, widget and formatter.
• The widget should be a button to open the icon library.
• The icons should use a naming convention to provide context that can be used as filters, that will avoid storing metadata for the icons in database or config, the pattern could be: solid-megaphone, outline-megaphone, duotone-megaphone
• A filter for the Icon library by Provider - The module or theme that provides the icons - FontAwesome, Claro or any other module or theme.
• A filter for the Icon library by Style - Solid, Outline, Duotone or any other word extracted from the first part of the name of the icon.
• The icon library should provide searchbox to find icons by name.
• The value for configuration or content entities should be the icon id

Let me know what do you think?

jidrone → created an issue.

jidrone → created an issue.

Thank you for your review and fixes.

Hello,

The existing patch has the following issues:

• The media targets provided are tightly coupled to other existing targets.
• It supports only image and file media types, so if you have a media type with a different name, it will not work.
• It assumes the source file of the media entity types always follows the pattern field_media_[media_type_id].
• The comments from the maintainer in #79 ✨ Add a mapping target to media field Needs work where not addressed.

This new MR, has the following approach:

• As minimal as possible changes to existing targets.
• All the code is in just one new target called Media for loose coupling.
• Maintainer comments addressed to ensure it is only applicable when media module is enabled
• It is only applicable when the media field has at least one media type that contains a source field of type file.
• Detects the applicable media type depending on the file extension.

I know there are still improvements to do and it will need tests, but this could be a better starting point.

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

catch → credited jidrone → .

I added a Clear content option in the toolbar, when clicked, it shows the autocomplete again to find a new node.

Moving to Needs Review, so more people can test.

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

jidrone → created an issue.

I added the --site-path option, which can be used to point the recipe to another site than the default.

Example:
php core/scripts/drupal recipe recipes/custom/basic_block --site-path subsite1
Assuming the path of the site is sites/subsite1.

jidrone → created an issue.

laura.gates → credited jidrone → .

smustgrave → credited jidrone → .

jidrone → created an issue.

Hi @solarDog,

Can you please share the error that appears.

Because those 4 are Drupal fields I would say the options are the following:

Option 1:

A formatter to be able to render the component as suggested in #4, which I think is out of the scope of this patch.

Option 2:

Use hook_theme_suggestions_HOOK to make all those fields to use a single template as suggested by Mark, the problem with this is how to set the icons properly, maybe by getting the field_name on each and assigning the icon accordingly.

Yes, I think the CKEditor integration should be in a separate module.

Created MR and moved to needs review

jidrone → created an issue.

Create MR and moved to needs review.

jidrone → created an issue.

I started a new issue fork and MR with multiple fixes:

• Changed references to CL components to SDC
• Removed variants functionality
• Removed CL Components dependency
• Multiple fixes to get the component schema

There are some things I didn't test yet, like the CK editor embed, I will move it to needs review for now.

I agree with @e0ipso, I was trying to make it work with the latest version of Drupal core and all other modules and I have found more things to fix, so instead of having a lot of small issues that don't work independently I would rename this for all the compatibility issues.

Once the patch get ready maybe we can create a 2.x version.

Merged PR and later I will add a link to a documentation page.

jidrone → created an issue.

Rate your excitement about SDC in core: 1 ... 10 | N/A

10

Rate your excitement about potential contrib extending SDC: 1 ... 10 | N/A

10

Rate our documentation: 1 ... 10 | N/A

7

Did you 1️⃣ refactor existing template into a component, or did you 2️⃣ write a component from scratch? 1 | 2

1 and 2.

Rate the helpfulness of error messages encountered: 1 ... 10 | N/A

10

Have you tried the Storybook integration? Yes | No

Yes (couldn't make it work yet, I will try later)

Any thoughts you would like to share? The sky is the limit

Thank you, for this great initiative and all your hard work.

It is not necessary to add a patch file when issues have been resolved via Merge Request, you can follow this guide https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-drupal/downloading-a-patch-file → , then add the patch from local file in the composer.json file.

It is not necessary to add a patch file when issues have been resolved via Merge Request, you can follow this guide https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-drupal/downloading-a-patch-file → , then add the patch from local file in the composer.json file.

Really good findings, I just fixed them, so it should be ready to review again.

As explained in the change record related with the Access Check → , only the content entities are affected by this change.

Most of the code changes in previous patches are related to that change record, but applied to config entities like ldap_server, user_role, etc.

So I think there is only one change related with Drupal 10 compatibility fixes, but it was already fixed on 🐛 Automated testing symfony/ldap 6.x issues Fixed

+++ b/ldap_servers/tests/modules/ldap_servers_dummy/src/FakeQuery.php
@@ -28,7 +29,7 @@ class FakeQuery implements QueryInterface {
+  public function execute(): CollectionInterface {

From my perspective all the things related to D10 compatibility are fixed, so I'm moving this to RTBC and other CS issues can be fixed in a separate issue.

I passes the test and code looks good.

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

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

Create MR and moved to needs review.

jidrone → created an issue.

Create MR and moved to Needs Review.

jidrone → created an issue.

Created MR and moved to Needs Review.

jidrone → created an issue.

I started a MR for this in 🐛 "Vote form/Results" does not work as expected in Manage display Needs work .

I started a MR for this in 🐛 "Vote form/Results" does not work as expected in Manage display Needs work .

After deeper testing I think the issue is related to how the entity theming is being managed.

There are other issues related like 💬 Manage form display issue Active and ✨ The order of output of fields before voting Active , I also had an issue using field group module.

I will start a merge request with an approach than can fix all of them.

After deeper testing I think the issue is related to how the entity theming is being managed.

There are other issues related like 💬 Manage form display issue Active and ✨ The order of output of fields before voting Active , I also had an issue using field group module.

I will start a merge request with an approach than can fix all of them.

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

apaderno → credited jidrone → .

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

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

Rerolled and moved to needs review.

Ideally, users should be able to use reusable blocks, just don't have permissions to edit them.

This is exactly what this patch is doing, they users without the permission are able to see the "Reusable" tab and add any of the existing reusable blocks, but they are not able to create new ones or edit the existing ones, so it is possible to control who manages reusable blocks.

And the reason it is being converted to regular block automatically is to improve the UX because as mentioned in the comment #4, it is really common that new Gutenberg users don't know they are editing a reusable block, so they change it accidentally.

I completely agree with @eelkeblok, if the issue already provide a MR is not needed/good to add patch and even worse without an interdiff.

The only difference I see between last patch and the MR is the following, which should not be responsibility of this issue.

+++ b/src/ClientFactory.php
@@ -27,6 +32,11 @@ class ClientFactory {
+  /**
+   * Redis default scheme: will not authenticate.
+   */
+  const REDIS_DEFAULT_SCHEME = 'tcp';

That being said, I tested the patch from the MR and it is working fine, so I just re-rolled, moved the new parameter to the end of some config arrays and fixed some CS.

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

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

Reverted the 2 cases that were not strpos replacements.

Hi,

I added the cases mentioned by @smustgrave and others that I found.

I'm only taking into account the following patterns as suggested in the description:

• strpos($a, $b) !== FALSE
• strpos($a, $b) === FALSE

I was not including:

• ($pos = strpos($input, '@', $pos)) !== false
• ($start = strpos($message, '{')) !== FALSE
• strpos($value, $condition['value']) === 0

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

jidrone → created an issue.

Pushed to the dev branch, thanks.

I just did it, sorry for the delay.

I just realized that this should be on Needs Review.

I needed to use the plain text formatter in a project, so I think at least it should not throw an error.

I have use this patch for more than a year and it is working ok.

I had the same issue while using this core patch #2924653: Allow preferred language selection for link field → , the proposed fix is good and work as expected.

The tests also passed with the right PHP version.

Hi @kunal_sahu

I don't see any difference between the MR you created and the patch.

Added deprecation test.

You are right, we need to deprecate the connection argument, I did so and also I started a draft change record.

if ($this->connection->query('SELECT COUNT(DISTINCT [vid]) FROM {node_field_revision} WHERE [nid] = :nid', [':nid' => $this->revision->id()])->fetchField() > 1) {

The problem is only with this line, so I think the way to get the existing revisions to redirect the user can be just changed to use the nodeStorage:

$this->nodeStorage->revisionIds($this->revision)

As I did in the patch.

I forgot to move it to NR.

After a deeper review I think the condition mentioned on this issue was misunderstood, because the intension is to know if there are existing revisions and redirect the user accordingly, if existing revisions go to the revisions history page, if not go to the node page.

It is also true that is needed to check if the user has access to the revisions history page, so we can take this opportunity to fix both things:

• Check if user has access to revisions history page.
• Replace the hardwired SQL query.

I improved the patch to remove the database service as well, because it is not used anymore for this test.

I'm improving the issue description and title.

Yes, you are right, it looks to go.

+++ b/core/lib/Drupal/Core/Form/FormValidator.php
@@ -335,6 +335,8 @@ protected function performRequiredValidation(&$elements, FormStateInterface &$fo
+      $message_arguments = ['%name' => $name];

Maybe we can also simplify this part to be something like:

$message_arguments['%name'] = $name;

What do think @longwave?

It is working fine and it makes sense based on the explanation found at https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attribut...

I tested on Drupal 10.0.2, and it is working as expected.

I think you can push the 3.x dev branch and once we test it is working we can create 3.0.x-beta1 tag.

All those files are generated from NPM command

Is not required to have a .module file.

I just added @ekes as maintainer and created 🌱 3.x Version Plan Active , we can continue the discussion there, please let me know what else can I do to help.

jidrone → created an issue.

Hi everyone,

Sorry for the late response I was really busy, I will add @ekes as maintainer and also we can create a plan issue to start discussing and dividing the effort into child issues for the new 3.x version.

After viewing the code I think both of you are right, the text in the summary was not the same than the patch and I think the suggestion from @jungle is good, because is not common in Drupal to make them bold or use quotes in those messages.

I suggest to change the description to remove the quotes and the strong tag as follows.

The submitted value FORGERY in Checkboxes element is not allowed.

Hi smustgrave,

Do you mean the issue description is not clear or it looks like was not updated?

Hi everyone,

I have been working in a module able to create a media element from the href attributes, it provides a Drupal Action and a feed tamper.

I think it is still on beta, but you can start trying the dev release on https://www.drupal.org/project/html_processors → .

Hi everyone,

I have been working on a new module able to convert HTML to Gutenberg, it provides a Drupal Action and a feed tamper.

I think it is still on beta, but you can start trying the dev release on https://www.drupal.org/project/html_processors → .

Created a patch with the new approach, in this case an interdiff makes no sense because the code is going into another direction.

Hi rpayanm,

That's the expected behavior after running the update, because it sets the formatter to match the hardcoded values to avoid unintended results on existing sites using that formatter.