Problem/Motivation

Currently in ConfigSingleExportForm to select and replace the field including its wrapper, we are using the CSS "has" selector: select('*:has(>select[name="config_name"])'). This works but is complex.

Proposed resolution

Add a data-drupal-wrapper-selector attribute to the wrapper element so that it can be directly targeted easily.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3603310

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

fathershawn created an issue. See original summary.

fathershawn opened merge request !16093

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Issue summary: View changes
fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Status: Active » Needs review

ConfigSingleExportForm working properly using the new selector to replace the config name select element and to return the export text area with the result.

smustgrave’s picture

smustgrave commented

Question could this be a breaking change for someone's custom build? Know it's a task does it need coverage or fact tests pass the coverage?

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented

Since this adds a new data attribute I don't think it would break anything.

The updated usage in ConfigSingleExportForm implicitly tests it. I'm happy to build a direct test if that's desired.

smustgrave’s picture

smustgrave commented

Mind rerunning the failed test job?

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Ran locally and ConfigDraggableListBuilderTest passes so not going to start a new pipeline.

godotislate made their first commit to this issue’s fork.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Needs work

Rebased and ConfigDraggableListBuilderTest has failed twice.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented

Thank you @godotislate! Fixed and pushed.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented

Okay so ConfigDraggableListBuilderTest is fixed and now passing but KernelTestHttpRequestTest although it passes locally. I don't see anything wrong there. Anyone see it?

nitinkumar_7’s picture

nitinkumar_7 commented

I don't immediately see anything in this change that would affect KernelTestHttpRequestTest. Since the failure is not reproducible locally, it may be worth checking the CI logs to see exactly which assertion is failing and whether its an intermittent issue.

nitinkumar_7’s picture

nitinkumar_7 commented

If the failure is unrelated to the changes in this MR, a rerun should help confirm that; otherwise the failing assertion should point us toward any unexpected side effects.

godotislate’s picture

godotislate
he/him
commented

Think it's unrelated. The test generates a random string to use as markup, but the generated random string might be sanitized when rendered and not be identical to the original string.

I'll open a new issue.

godotislate’s picture

godotislate
he/him
commented

Opened #3613115: KernelTestHttpRequestTest::testKernelTestRoute fails intermittently because random string is not sanitized for #15.

godotislate’s picture

godotislate
he/him
commented

For now, I re-ran the kernel test and it passed.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback appears to be addressed to me.