🇺🇸United States @smustgrave

With the release of 10.3-beta1 today and the validation updates that shipped with that think this is good to revisit.

A number of fixes were made in https://git.drupalcode.org/project/field_group/-/commit/a2a42e513639be08... but there is no release with it yet. Config inspector actually throws a ton of deprecation errors if using field_group

Did a release for 3.0.x but since that needs to be 10.3 for the validation for schema going to also include this in 2.0.x and do a release for that.

Yea I have a fix for bef I can push that helps until it’s fixed in core.

So this feature is working as expected. Like the simplicity of integrating with views and rendering control by bef

Came up again while testing 📌 Rewrite facets exposed filters to be native views filters. Active

Scratch that I needed to apply fix for 🐛 Argument #1 ($value) must be of type Countable|array, null given in count() (line 57 of core/lib/Drupal/Core/Render/Element/Radios.php). Needs work

So did some initial testing and ran into a bef fatal error that will address here 🐛 BEF php 8.1.2 / TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given Needs work

So added a facet for content types. The facet appears fine and settings appear to work. But the actual filter doesn't appear to work

"The submitted value article in the type element is not allowed."

smustgrave → changed the visibility of the branch 3445184-fatal-error-when to hidden.

Got bit by this nightmare again over in better_exposed_filters.

Cleaning up the tags but still leaving in NW for the tests.

Since the removal of jquery_ui slider is to use NoUiSlider instead don't think we need to point to another contrib module that does the same. No issue for users using both it as just another option, but don't want to confuse people with 2 options using the same approach.

Will try and plan one early next week. Waiting to see if anyone reviews the jquery removal tickets.

Okay so think this doesn't need an issue summary actually.

Tabbing through the hover is now activated

LGTM

#36 still needs to happen.

MR has a bunch of test failures.

Also if a new approach is going to be used then issue summary will have to be updated and fresh set of screenshots added. Moving to NW for that.

Didn't check the Unit failure to see if related but definitely something that will need test coverage.

I have the last 2 jquery_ui removal tickets up for review. Goal is to get those merged into 6.0.x, with the entry to composer still there. Then in 7.0.x remove completely.

Wound take a look at 📌 Remove dependency on jquery_ui_slider Needs review

Seems new approach still has appropriate coverage

1) Drupal\Tests\media\Unit\ResourceFetcherTest::testFetchTimeout
TypeError: Double\GuzzleHttp\Client\P1::request(): Return value must be of type Psr\Http\Message\ResponseInterface, null returned
/builds/issue/drupal-3228350/core/modules/media/src/OEmbed/ResourceFetcher.php:67
/builds/issue/drupal-3228350/core/modules/media/tests/src/Unit/ResourceFetcherTest.php:52
ERRORS!
Tests: 2, Assertions: 5, Errors: 1.

believe the change is good and title makes sense.

Tweaked versions slightly but assume that will have to be updated if it makes it in 10.3 or 10.4

There are a few issues around WEBP images in the queue, can you see if any of those match your problem.

Can the MR be updated to make sure tests and phpcs run fine.

Pushed a fix

Fine with this change. Committed to both 6.0.x and 7.0.x thanks!

Thanks committed to both 6.0.x and 7.0.x

Can the issue summary be completed please.

Proposed solution needs to be filled in

if a UI issue before/after screenshots should be included

Thanks!

I'd be lying if I said I fully understand the view config update (not one of the 5 haha) but running the test-only feature the tests for the update are failing as expected

1) Drupal\Tests\views\Functional\Update\EntityArgumentUpdateTest::testViewsFieldPluginConversion
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'entity_target_id'
+'numeric'

Can see the full list of expected test failures here https://git.drupalcode.org/issue/drupal-2640994/-/jobs/1604882

So believe the addition of that is good

Sounds good.

Believe this will need sub-maintainer approval for such a change, but could this functionality also live in a contrib module?

If approved will need an upgrade path and test coverage.

1000+ files is hard to review and crashes gitlab

But the 1 to 1 code addition to subtraction does indicate nothing more was added.

Been trying to help tickets I review by adding :void to tests (nitpicky stuff) but still large number here.

Spot checking the files I don't see anything off and tests are still green.

Believe this is good.

MR appears to have failures.

How come we are leaving " see https://www.drupal.org/project/drupal/issues/3443759 📌 LinkGenerator.php "System path is deprecated - use the route name and parameters" Needs review ." ? shouldn't that be removed.

Issue summary is still incomplete. Proposed solution is blank and should be filled in.

As a UI change there should be a before/after screenshot or gif in this case if possible.

Moving to NW for the suggestions made by @longwave.

If a new solution is being proposed issue summary should match.

Also before/after screenshots need to be added to the issue summary.

Question is this reproducible in just core and only with the contrib?

Believe we will need a test case to show the issue.

MR appears to be the patch that @larowlan uploaded here https://www.drupal.org/files/issues/2019-04-16/3039955.patch → just fyi for reviewers.

Feedback appears to be addressed.

I speak to @larowlan has he helps me out with core stuff. So if we want to look int component_block I can start with that if I can free up.

Will leave for maintainer but would close this one out. Pipeline is appearing fine https://git.drupalcode.org/project/simple_sitemap/-/commit/fecb649ee336a...

Was seeing it on a site I was using https://www.drupal.org/project/ui_suite_uswds_paragraphs → and the MR completely resolved the issues I was seeing.

Not sure the status of 📌 [2.0.0-alpha3] Fix config schema Needs work but this is a step better so maybe a follow up for the other 2 keys?

@mglaman should this be Needs work per #19?

New approach appears to work as well.

Maybe this?

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

Going to close out but if still experiencing an issue please reopen.

wonder if there is a better way to add the fields instead of scheduler_content_moderation_integration_entity_base_field_info()

Since we don't have 8.x-1.x marked supported can we confirm this is an issue in 2.x and/or 3.0.x?

Can this be converted to an MR. Also possible to add some test coverage please.

Rebased and tests are still green. Thanks for providing coverage!

No one has chimed in so maybe it is okay.

1) Drupal\Tests\comment\Functional\CommentPreviewTest::testCommentPreviewOnTranslatedNode
Behat\Mink\Exception\ResponseTextException: The text "Preview comment" was not found anywhere in the text of the current page.
/builds/issue/drupal-3145146/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3145146/vendor/behat/mink/src/WebAssert.php:293
/builds/issue/drupal-3145146/core/tests/Drupal/Tests/WebAssert.php:975
/builds/issue/drupal-3145146/core/modules/comment/tests/src/Functional/CommentPreviewTest.php:287
/builds/issue/drupal-3145146/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
ERRORS!
Tests: 4, Assertions: 118, Errors: 1.

Test coverage is there and the fix does solve the issue.

Going to mark.

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

Fixed this already for the 3.0.x branch but seemed to be failing due to the events changing from scheduler. Not sure why it didn't pick the define() calls there though.

So things I've merged

Dropped drupalci
No longer ignoring eslint, phpstan in the gitlab issue
Added some typehints since php8.1 is minimum requirement
Removed install hooks and tests related to them
Fixed the failing test (also replied to the 2.x ticket)

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

FYI unless there's a reason to keep I removed on the 3.0.x branch and fixed the errors so we are still green there.

If still experiencing this issue with 2.x branch or 3.0.x when I release that soon please reopen.

Just started a 3.0.x branch that will have D10 and D11 support only. Believe this one just got lost with time so going to close out for now.

Going to merge as it does solve the issue. 2.x branch is currently broken so that's not showing correct info.

Rebased for 2.x since 8.x-1.x is no longer supported.

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

Seems to need a manual rebase.

Reading the threads believe the MR is on track.

As far as tests go I also don't know if we test specific setup like that. Will mark then.

For good practice can the MRs be cleanup/hidden. There are currently 4 branche

Change record looks good so removing that tag.

Added release note to the summary

But MR appears to need a manual rebase.

So what's next steps needed for this one?

Since it's been a few days going to move to NW for the MR.

Leaving all tags as this should still be reserved for those who worked on it in Portland

This may help too https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

Issue summary is still incomplete

#8 mentions being able to replicate so need to verify that too

Tagging for follow up in #18

@Gauravvv you tagged for screenshots so moving to NW for those.

May need a rebase also.

Wonder if still an issue. I'm hesitant of adding back the updates as don't know the impact it will have for those on 5.0.x

Have paused the 4.1.x pipeline as I've got a lot of emails lately.

Thanks for the reminder. Setup a weekly pipeline for 5.0.x

Talking to a committer and this needs product manager sign off.

Fix should be in MR. #8 is just a use statement.

So we don't have 8.x-1.x listed as supported, do we want to fix it?

Rebased and fixed phpcs issue
Hiding patches and old MR (but then un-hid_

Will remove tests tag as test branch is showing failure.

But can we update the MR 1594 for 11.x and include tests too please.

smustgrave → changed the visibility of the branch 2865920-when-a-site to active.

smustgrave → changed the visibility of the branch 2865920-when-a-site to hidden.

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

Finally got around to this one

Did a fresh Umami install without the MR applied to verify column shifts on pages like /articles

Did a fresh Umami install with the MR already applied so the config imported
Visited /articles page to verify the layout is the same and responsive at various breakpoints

Then I did the same for the homepage as I saw that view was updated.
Also did not notice any issue.

Believe this is a good replacement.

So applied the MR locally.

Update hook ran just fine

I'm still able to update the settings without issue

The proposed schema changes believe make sense.

+1 from me.

Left a comment on the MR.

Don't think we will need an upgrade path as the types don't seem to be changing but not 100% on that.

This set was much easier to work with then the others haha

Reviewed the changes and don't see any issue.

Went to review but appears MR has some failures

Going to elevate to framework manager.

So for this one I applied the MR locally.

Looking at the files I went to a view page (Content for this test)
While adding a new field in the popup I did a resize and everything seemed to behave as before

I enabled layout builder and enabled for Article content type
Adding a section I dragged the off canvas region without issue.

Believe this one is good.

Believe I can see the issue when the page refreshes. On the "Page" tab. Some weird janky shift.

@YevKo as the reporter can you provide additional steps.

For good practice lets complete the issue summary.

Got it started.

Going to mark but maybe a follow up for layout builder performance should be opened? That's a question for the committer. But the change here looks good.

Screenshots appear to have been added and issue summary updated

Believe this is ready for accessibility.