Improve experimental theme/module installation warning

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

I've created MR !12710. Instead of the format listed, I believed it should be an Oxford comma, though this wouldn't be difficult to replace if not. Please could someone review.

I'm moving this back to needs work. I'm currently struggling to get tests working locally, so if someone would like to take this on, feel free.

I was able to get tests working locally and have fixed the issue and the pipeline now passes. Happy to move this back to NR. Please review MR !12710

arisha → changed the visibility of the branch 3086941- to hidden.

@arisha, can you clarify what you changed about your approach and why it might be preferred approach?

I'm moving to NW as there was already an MR with no explanation for the new one

I'll review the original since it is passing, I'll hide the new one unless @arisha replies with reasons to use the alternative.

nicxvan → changed the visibility of the branch 3086941-fix-the-message-grammar to hidden.

Looks like Drupal\Tests\system\Functional\Module\DependencyTest is failing.

Also one more minor suggestion on the mr.

Yeah, i was struggling with it, coming back to it today. Thanks for noticing that spelling mistake!

I think I finally got this all working! I created a new function to separate it all out. Please review!

I think this is ready thank you for working on this!

We have test coverage now for all three module install options. We don't have enough experimental themes to get coverage there as far as I know but the code is nearly identical.

I took a pass at assigning credit.

I did not credit @arisha because the issue was in needs review with a passing MR, a new MR was created without explanation or follow up so I'm not sure if that qualifies for credit.

Might be worth adding before and after screenshots too.

I'm working on the screenshots

NW because I think this will break translations. The comma-separated list was a compromise to inline the "proper" way of doing this. Thanks!

I wonder if there is a way to use the translated 'and'

Can we set it to a variable wrapped in t() and use that?

I've made a new commit, just need to do the tests which ill work on, but wont hate if someone gets to it before me. See this screenshot for format.

nicxvan → changed the visibility of the branch 11.x to hidden.

Ahh, thanks @nicxvan, i was wondering why an unrelated test was failing. I see they all passed the pipeline now, so ill move to NR

No problem!

Hiding the old screenshots I'll take be ones later unless someone reviews this before me.

I have not forgotten this! Still might be a bit before I can do the ux screenshots.

Ok I've finally had a chance to take new screenshots.

I think the bullet list is far better!

I'm going to do a quick review again of the code before marking this ready.

nicxvan → closed merge request !12716

Ok this is almost there! I took screenshots and I reviewed the code thoroughly.

We don't want to output raw html so I changed it to use item_list.

I added both fixes as suggestions, if you want to use them then you can follow this procedure:

1. look at the diff on the MR that gitlab displays.
2. if it looks right to you you can click either apply suggestion, or if there are more than one change you can click add suggestion to batch
3. once you have added all suggestions you want to apply you can click apply Apply suggestions and put a message

PHPCS might fail since it's a multiline change but I did my best to ensure the spacing is correct.

I ran the core/modules/system/tests/src/Functional/Module/DependencyTest.php locally to confirm it still passes.

I also confirmed that it does check the actual dependencies too.

Once these changes are in and it's green I'll mark it RTBC

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Moving this to needs review. It has passed (with warnings) on PHPUnit Tests for PHP 8.5.

I think this is ready again!