Single item configuration export form config_name does not have "- Select -" as it's first option

Needs reroll for 9.1.x as test code moved to core/modules/config/tests/src/Functional/ConfigExportUITest.php.

Re-rolled against 9.1.x. Kindly review a patch.

Patch #20 is a "test-only-patch". The actual fix is missing. Updating the patch here. Please review!

Thanks for the update.

When manually testing and going from a chosen configuration to the select option, it then shows false in the Here is your configuration: section. See screenshots.

And note the similar issue from @idebr: #2622468: Export single simple configuration name has no empty value, resulting in confusing UI

You can take a look at the patch there as well. One of these issues will need to be closed out but since this issue is slightly older and is being updated now, IMO it should be the other issue. Then those users should be noted here so they can get issue credit.

Fixing the bug in #24. Tested it manually as well. It now shows blank in the config text area on choosing 'Select'.
Also attaching the screenshots for reference.
Please review!

Fixing failed test.

Thanks for the update.

1) Manually tested and it worked as expected. Select option is available and chosen by default, switching to different configuration option shows the config for that, and switching back to select after that shows nothing in the Here is your configuration: box. See screenshots.

2) This still needs a proper code review and a comparison with #2622468: Export single simple configuration name has no empty value, resulting in confusing UI.

3) If this one is good-to-go, then the other issue should be closed out and all those issue participants should be added to this issue for issue credit.

Default:

Choosing an option:

Switching back to select:

I’ll review.

Can we get a test-only patch here that fails as expected, the one above passed - just to demonstrate this is fixed.

Can we use the '#empty_value' and '#empty_option' properties of select fields instead of adding the empty item to the array.

That will mean we don't need to think about this at all and can remove the existing code that sets the '' option.

Ok -- waiting for @larowlan's recommendation to be implemented before a complete review. However, I will second @Kirsten Pol's statement that it works as expected.

Adding a test only patch.
Used '#empty_option' of select field.
Please review the same.
Thanks

+++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
@@ -141,18 +142,26 @@ public function updateConfigurationType($form, FormStateInterface $form_state) {
-    // Determine the full config name for the selected config entity.
-    if ($form_state->getValue('config_type') !== 'system.simple') {
-      $definition = $this->entityTypeManager->getDefinition($form_state->getValue('config_type'));
-      $name = $definition->getConfigPrefix() . '.' . $form_state->getValue('config_name');
+    $config_type = $form_state->getValue('config_type');
+    $config_name = $form_state->getValue('config_name');
+    if (!empty($config_name)) {
+      // Determine the full config name for the selected config entity.
+      if ($config_type !== 'system.simple') {
+        $definition = $this->entityTypeManager->getDefinition($config_type);
+        $name = $definition->getConfigPrefix() . '.' . $config_name;
+      }
+      // The config name is used directly for simple configuration.
+      else {
+        $name = $config_name;
+      }
+      // Read the raw data for this config name, encode it, and display it.
+      $form['export']['#value'] = Yaml::encode($this->configStorage->read($name));
+      $form['export']['#description'] = $this->t('Filename: %name', ['%name' => $name . '.yml']);
     }
-    // The config name is used directly for simple configuration.
     else {
-      $name = $form_state->getValue('config_name');
+      $form['export']['#value'] = '';
+      $form['export']['#description'] = '';
     }
-    // Read the raw data for this config name, encode it, and display it.
-    $form['export']['#value'] = Yaml::encode($this->configStorage->read($name));
-    $form['export']['#description'] = $this->t('Filename: %name', ['%name' => $name . '.yml']);

with the new #empty option approach - are all these changes still needed? they may not be

@larowlan, yes these changes are required to fix the "false" value that was appearing in the configuration text area as mentioned by @Kristen Pol in #24.
These conditions are catering to that fix.

@larowlan, I'm able to confirm that @ridhimaabrol24's statement about that code being required to fix #24 prevents "false" from appearing in the area, and everything else about config export is working

However, i've uncovered something new. If you happen to switch to any other configuration type (that is, something that is not Simple Configuration), and then switch back to Simple Configuration, the "-- Select --" option no longer appears. I was attempting to provide a screenshot, but I'm out of review time for the day.

Please let me know if you're having difficulty reproducing and I'll provide screenshots later. Thanks!

Hi @larowlan
The options are being overriden in the callback function "findConfiguration". Hence empty_option doesn't work there. We need to include the select in the callback itself. Hence patch #28 works perfectly for that case. Please let me know your views on this and how can we proceed further.
Thanks

Those changes are out of scope, the attached interdiff is much simpler, and also works.

@larowlan, what changes are out of scope? Having "-- select --" be added back if the user switches to a different config type, and then back to "Simple Configuration"?

@partyka the changes I highlighted in #35 - #40 shows they're not required

@larowlan, I may not have communicated my observation well. I've taken a screen recording, and you can see it in the attached screen recording. I have applied your latest patch and I'm still seeing the "-- Select --" disappear after switching to a different configuration type.

Agree with #38/43, the empty option is there on page load but removed after an Ajax callback.

I tried to trace what is going on but got lost in the rendering system. I think what is happening is that the Select::processSelect() process callback (which is responsible for inserting the empty option) is not being called after ConfigSingleExportForm::updateConfigurationType() returns the element for update. I guess this might be a wider bug in Ajax select elements, maybe the empty option needs to be added in a prerender callback instead?

Ok, that also sounds like the added test isn't covering what we need it to.

Agree with #44. This seems like a larger issue with select elements having AJAX callbacks.

As suggested, I've moved the logic to add the empty option from Select::processSelect() to a pre render callback Select::preRenderSelect(). I also had to adjust the validation a bit to make it work

Let's see if this breaks anything

Looks like we can't just move it. We need this at both places or maybe I'm missing something here

Adding a quick test to demonstrate the issue

Thanks for working on this and well done on tracing and fixing this - you have more patience with debugging the Ajax render system than I do!

However, I feel like the original issue here has been hijacked and there is significant scope creep. Core committers tend not to like fixing two separate problems in the same issue where possible. Can you spin off a child issue where we can fix the Ajax issue on its own first, postpone this issue, wait for the child issue to be committed first, and then we can come back here and fix the original issue with the configuration export form?

Makes complete sense. Created the new issue here #3180011: Select's #empty_option gets removed on ajax callback