ENH Highlight the tabs which have validation errors (#1753)

silverstripe · May 22, 2024 · 65c0adb · 65c0adb
1 parent 8dc91bd
commit 65c0adb
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 23 deletions.
diff --git a/client/dist/js/bundle.js b/client/dist/js/bundle.js
diff --git a/client/dist/styles/bundle.css b/client/dist/styles/bundle.css
diff --git a/client/src/legacy/LeftAndMain.EditForm.js b/client/src/legacy/LeftAndMain.EditForm.js
@@ -121,7 +121,7 @@ $.entwine('ss', function($){
           'There are validation errors on this page, please fix them before saving or publishing.'
         );
 
-        const $editFormErrorBanner = $("#Form_EditForm_error");
+        const $editFormErrorBanner = $("#Form_EditForm_error, #Form_ItemEditForm_error");
 
         // Remove any existing invalid tab icons and screen-reader text
         this.find('.tab-attention, .tab-validation-error-sr').remove();
@@ -158,20 +158,28 @@ $.entwine('ss', function($){
           return;
         }
 
-        // Get the surrounding ss-tabset
+        // Get the tabs for this form
+        const $gridfieldTabs = this.find('.cms-content-header-tabs.cms-tabset-nav-primary li[role="tab"]');
         const $ssTabSet = $invalidTabPanes.closest('.tab-content').closest('.ss-tabset');
-        if ($ssTabSet.length) {
+        let getTabLi = null;
+        if ($gridfieldTabs.length > 1) {
+          // GridField logic
+          getTabLi = (invalidTabPaneId) => $gridfieldTabs.filter(`[aria-controls="${invalidTabPaneId}"]`);
+        } else if ($ssTabSet.length) {
+          // SiteTree logic
+          getTabLi = (invalidTabPaneId) => $ssTabSet.find(`#tab-${invalidTabPaneId}`).closest('li');
+        }
 
+        if (getTabLi !== null) {
           // Add invalid icons to tabs
           $invalidTabPanes.each((i) => {
             const invalidTabPaneId = $invalidTabPanes.eq(i).attr('id');
-            const $tabLi = $ssTabSet.find(`#tab-${invalidTabPaneId}`).closest('li');
+            const $tabLi = getTabLi(invalidTabPaneId);
             const $icon = $(`<i class="${iconClass}" title="${iconTitle}" aria-hidden="true"></i>`);
             const $screenReaderSpan = $(`<span class="tab-validation-error-sr sr-only">${iconScreenReaderText}</span>`);
             $tabLi.append($icon);
             $tabLi.append($screenReaderSpan);
           });
-
           // Set an alert message in the edit form error banner
           $editFormErrorBanner.attr('class', 'alert alert-danger');
           $editFormErrorBanner.html(alertMessageText);

diff --git a/client/src/styles/legacy/_style.scss b/client/src/styles/legacy/_style.scss
@@ -688,7 +688,10 @@ html, body {
 
 .validationerror .panel--padded .ui-tabs-nav {
   margin-top: -$panel-padding-y;
+}
 
+.validationerror .panel--padded .ui-tabs-nav,
+.validationerror .toolbar--north .ui-tabs-nav {
   .tab-attention {
     color: $state-draft;
     position: absolute;

diff --git a/tests/behat/features/validation.feature b/tests/behat/features/validation.feature
@@ -10,17 +10,34 @@ Feature: Form validation
     And the "Company" "Vitol" with "Category"="Other"
     And the "group" "EDITOR" has permissions "Access to 'Pages' section" and "Access to 'Test ModelAdmin' section" and "TEST_DATAOBJECT_EDIT"
     And I am logged in as a member of "EDITOR" group
-    And I go to "/admin/test/SilverStripe-FrameworkTest-Model-Employee"
-    And I press the "Add Employee" button
 
   Scenario: Field validation works as expected even if no validator is explicitly declared
+    Given I go to "/admin/test/SilverStripe-FrameworkTest-Model-Employee"
+    And I press the "Add Employee" button
     When I fill in "Name" with "any name"
     And I fill in "Email" with "invalid email"
     And I press the "Create" button
     Then I should not see "Saved Employee"
     And I should see "Please enter an email address"
 
-    And I fill in "Email" with "email@example.com"
+    When I fill in "Email" with "email@example.com"
     And I press the "Create" button
     Then I should see "Saved Employee"
     And I should not see "Please enter an email address"
+
+  Scenario: Validation in GridField highlights the tab where the error is
+    Given I go to "/admin/test/SilverStripe-FrameworkTest-Model-Company"
+    And I click "Walmart" in the "#Form_EditForm" element
+    When I fill in "Name" with ""
+    And I press the "Save" button
+    Then I should not see "Saved Company"
+    And I should see "\"Name\" is required"
+    And I should see "There are validation errors on this page, please fix them before saving or publishing."
+    And I should see an invalid tab icon on the "Main" tab
+
+    When I fill in "Name" with "new name"
+    And I press the "Save" button
+    Then I should see "Saved Company"
+    And I should not see "\"Name\" is required"
+    And I should not see "There are validation errors on this page, please fix them before saving or publishing."
+    And I should not see an invalid tab icon on the "Main" tab
diff --git a/tests/behat/src/AdminContext.php b/tests/behat/src/AdminContext.php
@@ -22,8 +22,8 @@ public function iShouldSeeAnInvalidTabIcon(string $not, string $tabLabel)
     {
         $id = str_replace('Main Content', 'Main', $tabLabel ?? '');
         $id = str_replace(' ', '_', $id ?? '');
-        $id = "tab-Root_$id";
-        $selector = "[aria-labelledby=$id] .font-icon-attention-1";
+        $id = "Root_$id";
+        $selector = "[aria-labelledby=tab-$id] .font-icon-attention-1, li[aria-controls=$id] .font-icon-attention-1";
         $hiddenSelector = ".ss-tabset-tabshidden $selector";
         $page = $this->getMainContext()->getSession()->getPage();
         if ($not) {

diff --git a/themes/cms-forms/templates/SilverStripe/Forms/CMSTabSet.ss b/themes/cms-forms/templates/SilverStripe/Forms/CMSTabSet.ss
@@ -5,15 +5,17 @@
 <%-- e.g. through LeftAndMain_EditForm.ss. --%>
 
 <div $AttributesHTML>
-	<% loop $Tabs %>
-		<% if $Tabs %>
-			$FieldHolder
-		<% else %>
-			<div $AttributesHTML>
-				<% loop $Fields %>
-					$FieldHolder
-				<% end_loop %>
-			</div>
-		<% end_if %>
-	<% end_loop %>
+	<div class="tab-content">
+        <% loop $Tabs %>
+            <% if $Tabs %>
+                $FieldHolder
+            <% else %>
+                <div $getAttributesHTML("class") class="tab-pane $extraClass">
+                    <% loop $Fields %>
+                        $FieldHolder
+                    <% end_loop %>
+                </div>
+            <% end_if %>
+        <% end_loop %>
+    </div>
 </div>