🇺🇸United States @bernardm28

Seems like this issue MR accomplishes the main goal of this issue.
It could always be improved later on a follow-up.

So it looks like the carousel does not work because we renamed
project.field_project_images to project.project_images but the fixture only finds one image with that name.
FYI updating a fixture within drupalPod is very slow.

I'm considering using https://biggerpicture.henrygd.me instead of making our own gallery since that supports multiple types.
Thoughts? I will add it to the MR tomorrow.

Ooooh, that's a great suggestion and a pretty cool module.
Though we already do something similar, in a slightly different way.

My issue is mostly around providing a way where we don't have to do img[alt=""], because a given image might need the warning in the future. Some components change their requirements as they serve a slightly different purpose.
I thought about a setting any given editor could toggle through a keyword or similar. Similar to the module above but with an Editoria11y exposed setting.
Editors can choose if the image in that location is decorative but if at a later point, it's not. They could remove the check or keyword and provide a way for Editoria11y to parse it again. That would be ideal.

I was using 'img[src*=".svg"], img[src*=".png"]' to ignore the extension, but I'm pretty sure editors will change their mind and have a png or .svg at some point that need the alt text warning even though it might not need it today.

That said, it's ok if you feel that's outside the current scope.

I will run through a few prototypes and see if I find something that might fit.

This probably comes with it's own challenges but will allow an override for those images an ambitious site builder deems decorative.

bernardm28 → created an issue.

This issue worked as expected on DrupalPod on Chrome.
We should open an issue up if it does not work in Edge and Safari but because this is an enhancement. I think supporting edge and safari would be could be a follow-up.
The feedback and accessibility concerns are valid and will be super useful as we make later enhancements but for now, I think this achieved the targeted outcome.

After looking into this issue, It seems to have accomplished the requirements. I tested it with drupalPod.
Hence, I'm setting it to RTBC.
I agree with @rkoller on most suggested changes but we are likely to address those in a follow-up ticket. I can open that.
Additionally, we will likely have to roll out the Drupal design systems everywhere once we are merged into core. So it might be postponed a little but it's coming. #331817 is not ready yet, but it is a good idea to reference it.

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

I was thinking on this items.
There is a button for add/install button but the items underneath i did not see them on the svelte file.
Those include

• Message if not compatible
• # Installs
• Stable
• Maintain
• Link to Drupal.org

tekNorah → credited bernardm28 → .

Since drupal already has a sidebar, I might copy some of its styles.

Some of the items on the right panel below do not yet exist. I imagine I should leave a placeholder.

bernardm28 → changed the visibility of the branch 3322594-project-detail to hidden.

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

After chatting with Mike Herchel about this. He recommended we get rid of the x altogether and use the standard HTML web search element instead of input.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/search

I wonder if tabindex="-1" would be good enough or would there be a better way.

Note: tabindex="-1" may be useful for elements that should not be navigated directly using the Tab key, but need to have keyboard focus set to them. Examples include an off-screen modal window that should be focused when it comes into view, or a form submission error message that should be immediately focused when an errant form is submitted.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabi...

hehehe thanks

bernardm28 → changed the visibility of the branch 1.0.x to hidden.

The test failure with the phpunit functional test is weird on this one.
Not sure that error is related to the changes here.

No worries, take your time. If anything, I might contribute back this one back after DrupalCon.
I worked around it by targeting the css classes. Which worked great.

bernardm28 → created an issue.

Patch https://www.drupal.org/files/issues/2023-09-28/2902164-149.patch → and 4.0.0-alpha5 seemed to work for the most part except inside group fields detail tabs, the parent and children would not hide even when the option was checked. However, I was able to hide regular paragraphs fields based another text list or drop down paragraph field.

That said, the current MR does not seem to work with paragraphs. https://git.drupalcode.org/project/conditional_fields/-/merge_requests/4...
My assumption is that https://www.drupal.org/project/conditional_fields/issues/2856720 ✨ Support for Inline Entity Form Fixed , the work related to supporting inline entities stepped on this.
I tried to apply the changes from #149 to the current MR on DrupalPod and most of the code overlap and highlights are related to the inline entities MR changes ✨ Support for Inline Entity Form Fixed . It might be tricky to support both and then layout builder not step on each other as the code in here seems to be growing in complexity and has started to lose some of it separation of concerns. Maybe a good problem to have since that mean more community will get behind it but a tricky one.

This should be ready to be reviewed.
I tested it with DrupalPod 10.1.x and the standard theme.

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

Looks good, thanks for checking and fixing those.

We are also experiencing this issue. While testing the current MR, 2902164-controlled-by-fields I noticed it also reorders the field's in the edit form when said field is inside of a details tab.
My paragraph had
field
field number 2
details tab
details tab number 2
embeded paragraph.

After applying the patch and trying to hide details tab number 2 with field number 2. The layout of the form switch to this.

field
field number 2
details tab
embeded paragraph.
details tab number 2

It did not hide the details tab number 2 but it did move it around.

Tested this with DrupalPod 10.1.x, stable theme. Seems to work as expected.
Searching and hitting enter multiple times does not remove any of the search filters.
I also tested this by removing one of the default test filters. Removing the maintained filter and searching repeatedly works as expected.

Thanks for the quick turnaround
The fix seems to work and this issue to be fixed by it.
We are looking to migrate away from extlink into linkpurpose so I might have some follow-up tickets.

bernardm28 → created an issue.

I created a new DrupalPod instance with Drupal 10.1 and the standard profile.
After adding text to the input field I was able to verify that #9 is not an issue anymore. As overlapping text still allows one to click the X and it works as expected by clearing the search results.

The test seems to work as expected checking whether there are results before searching for a string that has no results and comparing their outcome.

What if Instead of having it's own column it displays as an html subscript

It's an interesting idea and would save some time as one starts theming an entity.
Does it need to be there all the time? Maybe not, but it would be great to have it as a development option or have the ability to unhide it from somewhere on that table. Kind of hidden in plain site per se.

Also, I just noticed that clicking the original format as seen in the image below fixes it.

However, that setting does not get saved so upon opening the node the layout changes and it pics a different resolution.
So the image ckeditor plugin might be the buggy party here.

Same page without the issue.

removing style="height:1512px;width:2824px;" from <figure class="image ck-widget image_resized ck-widget_with-resizer ck-widget_selected" style="height:1512px;width:2824px;" contenteditable="false"

Seems like the issue comes from

without the issue

<figure class="image ck-widget image_resized ck-widget_with-resizer ck-widget_selected" style="height:1512px;width:2824px;" contenteditable="false"

removing style="height:1512px;width:2824px;" fixes the issue.

I can confirm this is an issue.
https://ddev.readthedocs.io/en/latest/users/quickstart/#__tabbed_4_2
I used the ddev quick start and then created a new basic page on Olivero's standard profile.

Seems to work as expected.

The image above is an example of what I would expect if I set the minimum and maximum values and go back into the config form. I would expect those values to load from the save config and allow me to change them again. Unfortunately, the form values will be empty no matter what.

The exposed filter worked as expected but the config form values for min and max was empty.

I modified Number.php to match what is used in NumericItemBase.php.

It should be working as expected now.

I tested this using drupalpod 10.1.x with demo_umami.

It looks to me like the min-max values are being saved but the plugin is not reading values that are already stored in config.
Even after you enter min 3 and max 30 and verify it works the form will load with empty values.

I used the /en/recipes view with the exposed filter of (field_cooking_time)

The up arrow seems to work well. I was able to increase the values with it and the min and max work as expected on the expose form but the plugin did not seem to read previously saved values.

Because it does not load the previously saved min max values on the config form it probably needs to go back to needs work.

With the help of the DrupalPod Chrome extension. I spun up a new Drupal site with Umami on it.
I did the following to test the module.

• First test
• Open the /recipes view in umami.
• Switch from the basic filter to better-exposed filters.
• Add Content: Difficulty as an exposed filter
• Save the view and check that /recipes load as expected. .
• Second test
• Open the /recipes view in umami.
• All of the steps from the first test.
• Change the reset button label text to reset test and check the always show reset button
• Save the view and check that /recipes load as expected. .
• Third test
• Open the /recipes view in umami.
• All of the steps from the first two tests.
• Change the reset button label text to reset test and check the always show reset button
• Change the submit button text Apply today and check the Input required field
• Swap the text desired to This is a test.
• Save the view and check that /recipes load as expected. .
• Fourth test
• Open the /recipes view in umami.
• All of the steps from the first 3 tests.
• Enable auto-submit with 100 ms with Hide submit button checked
• Save the view and check that /recipes load as expected.

Seems to be working as expected.

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

bernardm28 → created an issue.

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

It's fixed now.

Here is an image of how it looks after the rebase.
The grid does not look the same but that is likely just because this rebase does not include the new compile CSS assets.

cbfannin → credited bernardm28 → .

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

Looks good and works as expeted.

I spent some time looking at it today, I did not notice any visual regression.
Also, I added the FLDC2024 tag as I started working on this for Florida drupal camp contribution day.

bernardm28 → changed the visibility of the branch 3365435-update-markup-classnames-rebased-test to hidden.

bernardm28 → changed the visibility of the branch 3365435-update-markup-classnames to hidden.

Thanks for hosting and insights.

volkswagenchick → credited bernardm28 → .

kthull → credited bernardm28 → .

They were not the same last time I checked.

It works as expected. Thanks for the MR.

The last merge request works on my local, could It be a timeout issue with the functional PHP unit test, on drupal ci?

The issue seemed to be for most test errors that we had .project instead of .pb-project which could not be found.

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

froboy → credited bernardm28 → .

froboy → credited bernardm28 → .

mherchel → credited bernardm28 → .

johnpicozzi → credited bernardm28 → .

Not other than trying to figure out why it broke some tests.
Great feedback. I will add some notes in gitlab for most things it could probably be tweak.
That said, I'm not sure why it would still fail. Do you know? what trigger the test to fail? @kostyashupenko

It seems like it was solve as part of this https://www.drupal.org/project/gin/issues/3386007 🐛 Table sticky header appears when table not in viewport Fixed issue.
Thanks.

bernardm28 → created an issue.

volkswagenchick → credited bernardm28 → .

We also noticed teaser is included in a lot of twig comment blocks. Which if this is included should those also reference card as an example?
Maybe it good a good opportunity to update some of those comments as those tend to provide people with options.

+1 for this issue.

Laravel follows PSR-2 and that states https://www.php-fig.org/psr/psr-2/

There MUST NOT be a hard limit on line length; the soft limit MUST be 120 characters; lines SHOULD be 80 characters or less.

https://laravel-news.com/php-codesniffer-with-laravel
Even WordPress has more room there.

Lines should usually be no longer than 80 characters, and should not exceed 100 (counting tabs as 4 spaces). This is a “soft” rule, but long lines generally indicate unreadable or disorganized code.

https://developer.wordpress.org/coding-standards/wordpress-coding-standa...

Let's update this 10 year old issue.
https://www.drupal.org/project/drupal/issues/935284 →

volkswagenchick → credited bernardm28 → .

volkswagenchick → credited bernardm28 → .

kthull → credited bernardm28 → .

kthull → credited bernardm28 → .

Yeah its interesting the patch does that. Because the first one I made it manually with git and the second I download it doing this
https://git.drupalcode.org/project/drupal/-/merge_requests/4304/diffs.patch
I guess the second one does a patch with all the commits and not the latest commit.
But it does not matter. The if the MR is fine we are good to go.

I added the requested changes.

The MR is ready for review.
I did both the patch and MR but the MR is better go for that one.

Here is a patch for it.