Problem/Motivation

The UI needs to be checked for validation errors and display.

At least the cart shows a bogus/repeating message.
See image

The problem is most importantly that the message does not fit the checkbox space.

Proposed resolution

Only show the message above the table?
Compare to other bulk operation forms.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 interdiff-2529610-1-4.txt1.04 KBjuanse254
#4 validation_errors_shown-2529610-4.patch1.15 KBjuanse254
#1 validation_errors_shown-2529610-1.patch1.14 KBjuanse254
#1 Screen Shot 2015-07-10 at 09.58.57.png88.99 KBjuanse254
Screen Shot 2015-07-09 at 02.23.19 .png18.9 KBmiro_dietiker

Comments

juanse254’s picture

juanse254 commented
StatusFileSize
new Screen Shot 2015-07-10 at 09.58.57.png88.99 KB
new validation_errors_shown-2529610-1.patch1.14 KB

Adding Fix + Test and a screenshot as well.

juanse254’s picture

juanse254 commented
Status: Active » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/src/Tests/TMGMTUiTest.php
@@ -719,6 +719,10 @@ class TMGMTUiTest extends TMGMTTestBase {
+    //Check to see if no items are selected and the error message pops up.
+    $this->drupalPostForm('admin/tmgmt/cart', array(), t('Request translation'));
+    $this->assertText(t("No job items were selected."));

    Missing space after //.

    There is an assert to make sure a message is only shown once, let's use that. 

  2. +++ b/tmgmt.module
@@ -1064,7 +1064,7 @@ function tmgmt_source_add_to_cart_submit(array $form, FormStateInterface $form_s
-    $form_state->setErrorByName('items', t('No job items were selected.'));
+    $form_state->setError($form,t('No job items were selected.'));

    Missing space after ,

juanse254’s picture

juanse254 commented
Status: Needs work » Needs review
StatusFileSize
new validation_errors_shown-2529610-4.patch1.15 KB
new interdiff-2529610-1-4.txt1.04 KB

Changes from comment 3 taken into account.

sasanikolic’s picture

sasanikolic commented

Looks good, but still waiting for the tests to pass...

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work

Yeah, that single one is RTBC, but you should click through the UI to hit all these errors once and check if they appear correctly:

$ grep -R "setErrorByName" .
./sources/content/src/ContentEntitySourcePluginUi.php:      $form_state->setErrorByName('search[target_language]', $this->t('The source and target languages must not be the same.'));
./sources/content/src/Form/ContentTranslateForm.php:      $form_state->setErrorByName('languages', $this->t('You have to select at least one language for requesting a translation.'));
./sources/tmgmt_config/src/ConfigSourcePluginUi.php:      $form_state->setErrorByName('search[target_language]', $this->t('The source and target languages must not be the same.'));
./sources/tmgmt_config/src/Form/ConfigTranslateForm.php:      $form_state->setErrorByName('languages', $this->t('You have to select at least one language for requesting a translation.'));
./src/Form/JobForm.php:        $form_state->setErrorByName('translator', $translator->getNotAvailableReason());
./src/Form/JobForm.php:        $form_state->setErrorByName('translator', $translator->getNotCanTranslateReason($job));
./src/Form/SourceOverviewForm.php:      $form_state->setErrorByName('items', t("You didn't select any source objects"));
./src/Form/TranslatorDeleteForm.php:      $form_state->setErrorByName('tmgmt', t('This translator cannot be deleted as long as there are active jobs using it.'));
./src/Form/TranslatorForm.php:      $form_state->setErrorByName('plugin', $this->t('You have to select a translator plugin.'));

If not, fix them too.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Hm. The only one I see that is probably the same is SourceOverviewForm, the others should be OK since they reference a single form element.

I'd rather get this fixed than delayed on manually testing all those forms as some of those might not be easy to find/test manually.

juanse254’s picture

juanse254 commented

In the SourceOverviewForm the code related to create the error message is commented, created a followup here #2532716: Error message not showing when no source object selected .

juanse254’s picture

juanse254 commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Yes, looks good.

  • Berdir committed 2bb9018 on 8.x-1.x authored by juanse254 
    Issue #2529610 by juanse254: Validation errors shown repeatingly

Status: Fixed » Closed (fixed)

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