Problem/Motivation

On the single item config export form (/admin/config/development/configuration/single/export), one can change the configuration type in order to see configuration names that match the type, before then choosing a configuration name to be shown the item's export code. However, once a configuration name has been chosen, changing the configuration type leaves the export code in place, which can be confusing UX.

Proposed resolution

The proposed solution is to clear the export field value when the configuration type is changed. This will prevent confusion and allow the user to then choose a configuration name for the newly-select type.

Remaining tasks

A patch for this change is attached, and review/testing/comment would be appreciated. The change does not alter the UI or other behaviour on the form.

CommentFileSizeAuthor
#8 clear_config_field.patch1.56 KBsimonminter
clear_config_field.patch1.56 KBsimonminter

Issue fork drupal-3084436

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

simonminter created an issue. See original summary.

simonminter’s picture

simonminter commented
simonminter’s picture

simonminter commented
Category: Feature request » Task
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 8.7.7 » 8.7.x-dev
Component: configuration system » config.module
Status: Needs review » Reviewed & tested by the community
Issue tags: +Usability

This looks like a good idea and works as described - a nice simple usability improvement.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, clear_config_field.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Reviewed & tested by the community

Unrelated fail: #3045612: Random test failure in MediaStandardProfileTest::testMediaSources

Back to RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests
  1. It'd be great to write a test for this. For more information about writing tests in Drupal 8 see the following links:
    1. https://www.drupal.org/docs/8/testing
    2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.7.x
  2. +++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
@@ -84,6 +84,8 @@ public function getFormId() {
+    $form['#prefix'] = '<div id="config-form-wrapper">';

    As this is for javascript interaction only we need to add a js- prefix.

simonminter’s picture

simonminter commented
StatusFileSize
new clear_config_field.patch1.56 KB

An updated patch is attached here, which adds a js- prefix to the element ID as suggested.

I haven't written a test due to limitations of time and knowledge. If anybody out there fancies writing one to test this new functionality that'd be great, and a very useful thing for me to learn from!

Version: 8.7.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

alison’s picture

alison
she/her
commented
Related issues: +#2710143: Single item: Configuration type change does not update Configuration name config content if one exists with the same name

Agree -- super annoying bug, and catches people in traps sometimes! I was just explaining to a colleague how to export things, and fortunately he noticed that the "filename" below the export field hadn't changed.

-------
This issue is a duplicate of #2710143: Single item: Configuration type change does not update Configuration name config content if one exists with the same name -- not sure which issue to keep, someone else can decide.

For now, I'm just posting the same "YAAASSSS" comment on each issue + mentioning the other 👋

simonminter’s picture

simonminter commented

Thanks @alisonjo315 – didn't realise this was a duplicate. The other ticket was raised before mine so I guess that one should be the 'real' one, although this one has my patch included…

I didn't write a test for my patch due to limitations mentioned above. So if anybody out there could do that, this might edge closer towards being a solution!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Thanks for finding the duplicate. Usually we keep the older issue, but in this case there is a patch here and not there, so I've closed the other issue as duplicate of this one.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 8.9.x-dev » 9.3.x-dev

I'm also bumping this to 9.3.x as changes will be implemented there first, core committers will decide if the issue is necessary to backport to older versions.

longwave opened merge request !629

longwave opened merge request !630

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
Issue tags: -Needs tests

Opened a merge request, added a test, and a separate test-only branch without the fix to prove that the fix solves the issue.

simonminter’s picture

simonminter commented

@longwave Thanks very much for keeping this moving forward!

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

- Test fails for test-only MR
- TestBot is merrily green for the actual MR.
- Comments in #7 addressed.
- Code changes make sense
======================== +
- RTBC

spokje’s picture

spokje
Primary language Dutch
commented
Issue tags: +Bug Smash Initiative
spokje’s picture

spokje
Primary language Dutch
commented
Issue tags: -Bug Smash Initiative

catch credited pameeela.

catch credited webel.

catch’s picture

catch
he/him
Primary language English
commented

Adding credit from the duplicate issue.

  • catch committed caac9c9 on 9.3.x 
    Issue #3084436 by longwave, simonminter, Spokje, alexpott, alisonjo315,...

  • catch committed 6818c41 on 9.2.x 
    Issue #3084436 by longwave, simonminter, Spokje, alexpott, alisonjo315,...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

alison’s picture

alison
she/her
commented

Yay thank you everyone!!

Aaaany chance it could be made on 8.9......? 🤞🤞

simonminter’s picture

simonminter commented

Glad it made it through, thanks everybody! Oh and +1 for 8.9!

alison’s picture

alison
she/her
commented

(I'd be happy to help write a patch for 8.9 if maintainer folks say it could be committed, but I know not all issues/fixes get to be backported.)

pameeela’s picture

pameeela commented

8.9 is security fixes only now, so only issues that help people upgrade would be eligible for backport.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.