Field plugin settings edit button doesn't show unless there is a settings summary

Makes sense to me. Tagging for a usability review.

You could simply check if ($plugin->getSettings())

Other than that, why not. Please check whether the phpdoc for the summary() methods in the widgets an formatters interfacesneed to be updated ?

Can we leave checking documentation to a separate issue?

Not really :-). If the changes here make existing docs incorrect/obsolete, they should be updated accordingly in the patch too.
I.m only talking about checking the existing docs for settingsSummary() and settingsForm() in FormatterInterface because they might mention the current behavior (button only if summary not empty). Maybe they don't and everything's fine, it's just that I'm away from my coding env and checking the issue queues with my phone, so it's not easy for me to check the code myself :-) - but TBH I'd be surprised if the current behavior isn't docimented anywhere.

Yeah, there was a mention in the interface.

Thanks @swentel, forgot about this one.
Right, should be good then :-)

We should have a test for this so we don't break this during future refactorings.

Going to raise this to major. The settings form also doesn't work anymore if you move a field from hidden to content, you can't toggle the settings button as long as you haven't saved the the display.

Refocusing, opened #2084371: Settings form does not work when moving from hidden to content

Now with tests

Hmm the test is actually, wrong, one second.

Fix

This is green.

Looks good.

Patch no longer applies.

rerolled

Ok, this should be better, too lazy for interdiff though.

Dear me

#25: field_plugin_settings_edit-button-2029857-25.patch queued for re-testing.

The function signature for viewElements was changed.

#28: 2029857-28.patch queued for re-testing.

After the big Field renames ...

Committed 069902e and pushed to 8.x. Thanks!

Added missing @file doc block during commit

--- a/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/field/formatter/TestFieldEmptySettingFormatter.php
+++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/field/formatter/TestFieldEmptySettingFormatter.php
@@ -1,5 +1,10 @@
 <?php

+/**
+ * @file
+ * Contains \Drupal\field_test\Plugin\field\formatter\TestFieldEmptySettingFormatter
+ */
+
 namespace Drupal\field_test\Plugin\field\formatter;

Hello

I've been testing this and I found an annoying behavior with patch committed in #31.

Problem is that it is checking this:

+++ b/core/modules/field_ui/lib/Drupal/field_ui/DisplayOverviewBase.php
@@ -355,6 +355,8 @@ protected function buildFieldRow($field_id, FieldInstanceInterface $instance, En
+        }
+        if ($plugin->getSettings()) {
           $field_row['settings_edit'] = $base_button + array(

Plugin can have settings, but not form settings. In that case, the settings button is shown, but once clicked, nothing happens.

It should check that there are an available setting form for that plugin instead of the existence of a settings array.

Steps to reproduce:

1. Create a field which haa more than one available formatter, one of them ithout settings form (IE: Number float field)
2. Select the formatter which has setings form (IE: Default formatter) and configure its settings
   
3. Select the formatter which does not have configuration form (IE: Unformatted). After the AJAX, you'll see something like this:
   
4. Click on the configuration button, and check that nothing happens

Attaching patch that checks if the plugin has a config form instead of settings array.

Regards.

That's weird. When you select a different formatter, the whole row should be updated, and the presence of the "settings" button re-evaluated against the new formatter.

Changing status. Let's see testbot...

This seems to be just a bug.

minor edit, plugin instead of formatter.

Hello

I was talking yesterday with yched about this issue, and he suggested me to display or not edit plugin link depending on getPluginDefinition() provides plugin settings or not. It works unless hook_field_formatter_settings_form_alter() adds some form elements to a plugin that does not provide settings. In that case, edit plugin link is not displayed and you can't edit settings provided by hook_field_formatter_settings_form_alter().

Attaching 3 files, test-only, plugin-definition approach and settings form approach.

Any thoughts?

Regards.

It works unless
hook_field_formatter_settings_form_alter() adds some form elements to a
plugin that does not provide settings

Currently, a module that implements hook_field_formatter_settings_form_alter() typically also alters the formatter definition to add the corresponding settings - or else the form inputs wouldn't get saved.

This might change after #2005434: Let 3rd party modules store extra configuration in EntityDisplay / #2136197: Move field/instance/widget/formatter settings out of annotation / plugin definition, but let's see when those happen.

2029857-37-plugin-definition.patch simply fixes the implementation of the approach committed earlier, I 'd rather stick with that for now ?

We should add a comment explaining why $plugin->getSettings() isn't appropriate here.

OK, let's see if now we have it :)

Now checks getPluginDefinition(), created a new field_test formatter without any setting to gives test support.

Regards.

Looks good. Thanks !

Committed bf3ac2f and pushed to 8.x. Thanks!

Fixed the following during commit.

git diff ./core/
diff --git a/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/Field/FieldFormatter/TestFieldNoSettingsFormatter.php b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/Field/F
index 2eb76e0..1d3d150 100644
--- a/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/Field/FieldFormatter/TestFieldNoSettingsFormatter.php
+++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/Field/FieldFormatter/TestFieldNoSettingsFormatter.php
@@ -3,7 +3,7 @@
 /**
  * @file
  *
- * Contains \Drupal\field_test\Plugin\field\formatter\TestFieldNoSettingsFormatter.
+ * Contains \Drupal\field_test\Plugin\Field\FieldFormatter\TestFieldNoSettingsFormatter.
  */
 namespace Drupal\field_test\Plugin\Field\FieldFormatter;