Problem/Motivation

When submitting the configuration form at admin/config/content/entity_browser/FOOBAR/edit or admin/config/content/entity_browser/FOOBAR/widgets, no confirmation message appears. A minor but still.

Proposed resolution

Add a message that confirms that settings were changed.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#19 entity-browser-remove-duplicate-trait-3038292-19.patch491 bytesoknate
#12 entity-browser-confirmation-message-3038292-11.patch19.24 KBoknate
#11 entity-browser-confirmation-message-3038292-11.patch19.24 KBoknate
#11 3038292-interdiff--9-11.txt915 bytesoknate
#9 entity-browser-confirmation-message-3038292-9.patch19.83 KBoknate
#8 entity-browser-confirmation-message-3038292-8.patch19.59 KBoknate
#7 entity-browser-confirmation-message-3038292-7-v3.patch14.8 KBoknate
#6 entity-browser-confirmation-message-3038292-6-v3.patch15.01 KBoknate
#4 entity-browser-confirmation-message-3038292-4-v3.patch14.56 KBoknate
#4 entity-browser-confirmation-message-3038292-4-v2.patch4.44 KBoknate
#4 entity-browser-confirmation-message-3038292-4.patch1.4 KBoknate
#2 entity-browser-confirmation-message-3038292-2.patch1.37 KBoknate
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Primsi created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Active » Needs review
FileSize
entity-browser-confirmation-message-3038292-2.patch1.37 KB
Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Needs review » Needs work

Thanks. Maybe we could use the messenger service, given that it's already available in entity forms.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
entity-browser-confirmation-message-3038292-4.patch1.4 KB
entity-browser-confirmation-message-3038292-4-v2.patch4.44 KB
entity-browser-confirmation-message-3038292-4-v3.patch14.56 KB

Here are three patches that use the messenger service.

1) First one just uses messenger trait, which will call \Drupal::messenger()

2) Second one injects messenger service into the two forms

3) Third one replaces drupal_set_message() throughout the entity_browser module.

Status: Needs review » Needs work

The last submitted patch, 4: entity-browser-confirmation-message-3038292-4-v3.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
entity-browser-confirmation-message-3038292-6-v3.patch15.01 KB

Fixing v3.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
entity-browser-confirmation-message-3038292-7-v3.patch14.8 KB

Reroll of #6 after #3033289: Remove unused constructor argument in EntityReferenceBrowserWidget

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Component: Miscellaneous » User interface
FileSize
entity-browser-confirmation-message-3038292-8.patch19.59 KB

Adding test coverage.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
entity-browser-confirmation-message-3038292-9.patch19.83 KB

Reroll

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Needs review » Needs work 
+++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
@@ -401,6 +406,9 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
+    if (empty($element['#attached']['drupalSettings']['entity_browser'])) {
+      return $element;
+    }

Seems unrelated?

Apart from that seems quite ok to me. Tested this manually, worked quite alright.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
3038292-interdiff--9-11.txt915 bytes
entity-browser-confirmation-message-3038292-11.patch19.24 KB

Updated.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
entity-browser-confirmation-message-3038292-11.patch19.24 KB

Running test against 8.x-1.x branch.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

  • oknate committed 523b363 on 8.x-2.x 
    Issue #3038292 by oknate: Adding confirmation message after saving...

  • oknate committed 8353d87 on 8.x-1.x 
    Issue #3038292 by oknate: Adding confirmation message after saving...
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

Committed. Thanks for the idea/review, Primsi.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Reviewed & tested by the community » Fixed
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Fixed » Needs work

Looks like I introduced a minor bug:
https://www.drupal.org/pift-ci-job/1234548

FormBase already has MessengerTrait.

class FormElementTest extends FormBase {

  use MessengerTrait;
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
entity-browser-remove-duplicate-trait-3038292-19.patch491 bytes
Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Good catch.

  • oknate committed 7644b4e on 8.x-2.x 
    Issue #3038292 by oknate: Removing redundant use statement

  • oknate committed 1b7f33f on 8.x-1.x 
    Issue #3038292 by oknate: Removing redundant use statement
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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