🇺🇸United States @markie

Added MR that adds default text to the header demo component. Tested on Drupalpod and it works as requested, but if someone else could review the MR and then set to RTBC that would be great

My earlier issue was a semantic issue in the component.yml.. Merge request created that gives a sample of this change. After review and RTBC this should be considered completed.

I pushed the latest dev into Drupalpod and updated the Heading demo component to be an enum. The form rendered properly, but I got an error when clicking submit. Setting to Needs Work to review more later.

This blocks the D11 upgrade.

This doesn't necessarily need work, but the dependency on drupal/cl_editorial fails download in Drupalpod and locally

Started playing with a few options in DrupalPod but decided to move to my local to see if I can solve my personal issue.

Not all my personal issues... that's too much to ask

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

Reviewing with Dubbot team to make sure the tabs are returning properly.

damienmckenna → credited markie → .

smustgrave → credited markie → .

smustgrave → credited markie → .

markie → created an issue.

markie → created an issue.

teknorah → credited markie → .

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

Merged into the 2.x branch. Not sure why 2.x is not coming up as the issue parent.. will investigate.

This has been tested internally but if anyone wants to take a look at the changes and suggest improvements I am always open to that.

damienmckenna → credited markie → .

markie → created an issue. See original summary → .

markie → created an issue.

markie → created an issue. See original summary → .

markie → created an issue.

Tested in Drupalpod with D11.. RTBC

Closing in favor of #3428925

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

Merged in.. tagging and cutting a release now

Simple file addition just needs a rubber stamp.

MR has been merged and ready for a D11 release. Thanks for your contribution.

MR merged in and issue closing. Thanks for your contribution

Thinking that this was a different module... I created an MR so I could merge it in more easily.. but then.. I can't... Please make sure WalkingDexter gets all the credits.

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

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

There is nothing in this modules codebase that requires the menu_link_config module. I know our internal test instance uses that module, but should this issue be here? Or am I missing something?

markie → created an issue.

Found a small issue with a change in the configFormBase constructor that doesn't seem to be picked up. Tested in DrupalPod successfully but since Dubbot isn't looking for the test instance that is as far as I can get without releasing this. Can someone please review the MR and mark RTBS before I commit?

Looks like the MR is merged and dev codebase looks correct. All we need is a new release... marking as fixed..

damienmckenna → credited markie → .

outdated

Closing this as outdated.

The MR patch does work, but I am not a fan of that big if statement. See my comment in the MR.

Please credit slayne40 for the commit. It was their code.

markie → created an issue.

Looks like you added another commit to the dev branch to take care of this.. marking as needs review.

Oh weird.. This must be because of my branch issues.. I think I'll close the MR, and create a new one in the issue branch... will do that tomorrow or so..

I am surprised you are doing this in the dev branch. Creating an issue branch and MR is recommended.

That being said, the switch case on line 112 will over-write this change. You will probably want to have that logic concatenate the array message to the description:

        case 'array':
          $form['component']['fields'][$field]['#description'] .= $this->t('Each value must be separated by a space.');
          break;

+1 on this issue since it causes a WSOD with not much detail on which component is missing the definition. SDC documentation states all props are optional so I imagine most won't define attributes since that's just handed down by default.

I am not seeing anything in this MR that changes the `Drupal\sdc\ComponentPluginManager`... I that something that changed from the 1.0.x branch and the 1.1.x branch? This MR should be able to be cherry picked to the 1.0.x branch without changing the plugin.

rachel_norfolk → credited markie → .

markie → created an issue.

I made some more updates including pulling in the latest in the 1.1.x branch. I was able to verify this in Drupalpod by installing the sdc_styleguide_demo_components and removing the title and description from the card.component.yml file, clearing cache, and then reloading the styleguide. No notice or warnings appeared.

markie → created an issue.

I did some updates to the forms since this was giving the errors as well. I think the checks for empty title and description in properties should be done since they are considered "optional" in the component.yml file.

PS: Did the work in the issue branch first, then realized the MR wasn't against that branch, so did a local cherry pick and pushed to the proper branch.

This means the component.yml file is not properly populated.

The issue I think is that SDC documentation says that the component.yml file content is all optional, so some folks just create the file and leave it blank (me.. I'm some folks). There should be checks in the form and demos generators to assure the arrays are populated. Of course, it would be better for developers to properly fill out the component.yml to spec.

FYI: The patch generated by the MR is not applying to the @alpha5 instance

Steps to reproduce:

• Save plain diff of MR locally
• composer update drupal/conditional_fields
• observe error

There are rejections in:

• conditional_fields.module
• src/Form/ConditionalFieldDeleteFormTab.php
• src/ConditionalFieldsElementAlterHelper.php.rej
• src/ConditionalFieldsFormHelper.php.rej

Patch #190 applies cleanly. I imagine its because the target branch is 4.x instead of the @alpha tag..

Here is my use case:

I have a paragraph we can call "Container" with the following fields:

• Style: Image, Icon
• Cards: Entity Reference to a Card Paragraph

The "Card" Paragraphs has the following fields:

• Image: Image File
• Icon: Preset list of possible icons
• Text: Text

I want the container to control whether the Site Builder can use Images or a preset list of icons. I want this consistent for ALL cards inside a container which is why the style is in the container paragraph instead of the card itself. So I would Hide the icon field if the style is Image and vice versa for the Image field. I can select the "parent_field_name", but can not specify the name, and most importantly, can not trigger based on the value of the specified field.

I am trying to work this out programmatically for the time being but thought writing out a use case would be helpful for the module.

HTH

damienmckenna → credited markie → .

dan2k3k4 → credited markie → .

john cook → credited markie → .

This is working. I tested it but there is a warning if the column does not exist in the webform.

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

I would recommend supporting this issue over the other one.

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

You really do not need to add patches to an issue if there is an MR. In the MR block, there is a "plain diff" link which has a URL to the patch version of the MR. That could be seen as credit gaming.

dan2k3k4 → credited markie → .

latest version has abillity to strip certain tags.

@krzysztof those config entries were removed. Instead of adding them back, we should have this issue create an update hook to remove them from properly.

Updated the summary per your request. Please let me know if this solves it.

Just pushed a release for D10 support. Should be built in a few. Closing this issue.

MR looks good.. merged

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