Merge pull request #57 from AndyButland/bugfix/remove-duplicate-valid…

…ation-summary-entries

Remove duplicate validation summary entries

Pages/Demos/Checkboxes.cshtml

@@ -7,8 +7,12 @@
 
 <partial name="Shared/_StatusMessage" model="Model.StatusMessage"/>
 
+<div asp-validation-summary="All">
+    <span>Please correct the following errors</span>
+</div>
+
 <fieldset>
-    <legend>Required ASP.NET Checkboxes with hidden input</legend>
+    <legend>Required ASP.NET Checkboxes (with hidden input) and Radio buttons</legend>
 
     <form method="post">
         <div class="form-field">
@@ -53,6 +57,29 @@
             }
         </div>
 
+        <div class="form-field">
+            <p>
+                Similarly, with a required radio button list, one element should be checked.
+            </p>
+            @foreach (var fruit in Model.Fruits) {
+                <input name="SelectedFruit"
+                       type="radio"
+                       value="@fruit"
+                       data-val="true"
+                       data-val-required="Please select at least one fruit"
+                       data-rule-required="true"
+                       data-msg-required="Please select at least one fruit"
+                       @if (Model.SelectedFruit == fruit) {
+                           <text>checked</text>
+                       }/>
+                <label>@fruit</label>
+            }
+            <span asp-validation-for="SelectedFruit"></span>
+            @if (Model.SelectedFruit != string.Empty) {
+                <em class="results">Selected fruit: @Model.SelectedFruit</em>
+            }
+        </div>
+
         <input type="submit" value="Submit"/>
     </form>
 </fieldset>

Pages/Demos/Checkboxes.cshtml.cs

@@ -26,6 +26,12 @@ public IActionResult OnPost()
 
     public IReadOnlyList<string> Animals = new List<string> { "Dog", "Cat", "Fish" };
 
+    [BindProperty]
+    [Required]
+    public string SelectedFruit { get; set; } = string.Empty;
+
+    public IReadOnlyList<string> Fruits = new List<string> { "Apple", "Banana", "Strawberry" };
+
     public class InputModel
     {
         public bool IsChecked { get; set; }

Pages/Index.cshtml

@@ -6,7 +6,7 @@
 }
 <h1>Other Demos</h1>
 <ul>
-    <li><a asp-page="Demos/Checkboxes">Checkboxes</a></li>
+    <li><a asp-page="Demos/Checkboxes">Checkboxes and Radio Buttons</a></li>
     <li><a asp-page="Demos/SubmitButton">Submit Button</a></li>
     <li><a asp-page="Demos/RemovedInputs">Removed Inputs</a></li>
 </ul>

dist/aspnet-validation.css

@@ -17,3 +17,7 @@
     color: red;
     font-style: italic;
 }
+
+.validation-summary-valid span {
+    display: none;
+}

dist/aspnet-validation.js

@@ -957,11 +957,28 @@ var ValidationService = /** @class */ (function () {
         if (!Object.keys(this.summary).length) {
             return null;
         }
+        var renderedMessages = [];
         var ul = document.createElement('ul');
         for (var key in this.summary) {
+            // It could be that the message we are rendering belongs to one of a fieldset of multiple inputs that's not selected,
+            // even if another one in the fieldset is. In that case the fieldset is valid, and we shouldn't render the message.
+            var matchingElement = this.elementByUID[key];
+            if (matchingElement instanceof HTMLInputElement) {
+                if (matchingElement.type === "checkbox" || matchingElement.type === "radio") {
+                    if (matchingElement.className === this.ValidationInputValidCssClassName) {
+                        continue;
+                    }
+                }
+            }
+            // With required multiple inputs, such as a checkbox list, we'll have one message per input.
+            // It's one from the inputs that's required, not all, so we should only have one message displayed.
+            if (renderedMessages.indexOf(this.summary[key]) > -1) {
+                continue;
+            }
             var li = document.createElement('li');
             li.innerHTML = this.summary[key];
             ul.appendChild(li);
+            renderedMessages.push(this.summary[key]);
         }
         return ul;
     };
@@ -973,6 +990,7 @@ var ValidationService = /** @class */ (function () {
         if (!summaryElements.length) {
             return;
         }
+        // Prevents wasteful re-rendering of summary list element with identical items!
         // Using JSON.stringify for quick and painless deep compare of simple KVP. You need to sort the keys first, tho...
         var shadow = JSON.stringify(this.summary, Object.keys(this.summary).sort());
         if (shadow === this.renderedSummaryJSON) {
@@ -983,8 +1001,13 @@ var ValidationService = /** @class */ (function () {
         var ul = this.createSummaryDOM();
         for (var i = 0; i < summaryElements.length; i++) {
             var e = summaryElements[i];
-            e.innerHTML = '';
-            if (ul) {
+            // Remove existing list elements, but keep the summary's message.
+            var listElements = e.querySelectorAll("ul");
+            for (var j = 0; j < listElements.length; j++) {
+                listElements[j].remove();
+            }
+            // Style the summary element as valid/invalid depending on whether there are any messages to display.
+            if (ul && ul.hasChildNodes()) {
                 this.swapClasses(e, this.ValidationSummaryCssClassName, this.ValidationSummaryValidCssClassName);
                 e.appendChild(ul.cloneNode(true));
             }
@@ -1013,10 +1036,10 @@ var ValidationService = /** @class */ (function () {
             var inputs = input.form.querySelectorAll("input[name=\"".concat(input.name, "\"]"));
             for (var i = 0; i < inputs.length; i++) {
                 this.swapClasses(inputs[i], this.ValidationInputCssClassName, this.ValidationInputValidCssClassName);
+                var uid = this.getElementUID(inputs[i]);
+                this.summary[uid] = message;
             }
         }
-        var uid = this.getElementUID(input);
-        this.summary[uid] = message;
         this.renderSummary();
     };
     /**
@@ -1037,10 +1060,10 @@ var ValidationService = /** @class */ (function () {
             var inputs = input.form.querySelectorAll("input[name=\"".concat(input.name, "\"]"));
             for (var i = 0; i < inputs.length; i++) {
                 this.swapClasses(inputs[i], this.ValidationInputValidCssClassName, this.ValidationInputCssClassName);
+                var uid = this.getElementUID(inputs[i]);
+                delete this.summary[uid];
             }
         }
-        var uid = this.getElementUID(input);
-        delete this.summary[uid];
         this.renderSummary();
     };
     /**

src/aspnet-validation.css

@@ -17,3 +17,7 @@
     color: red;
     font-style: italic;
 }
+
+.validation-summary-valid span {
+    display: none;
+}

src/index.ts

@@ -960,11 +960,31 @@ export class ValidationService {
             return null;
         }
 
+        let renderedMessages = [];
         let ul = document.createElement('ul');
         for (let key in this.summary) {
+
+            // It could be that the message we are rendering belongs to one of a fieldset of multiple inputs that's not selected,
+            // even if another one in the fieldset is. In that case the fieldset is valid, and we shouldn't render the message.
+            const matchingElement = this.elementByUID[key];
+            if (matchingElement instanceof HTMLInputElement) {
+                if (matchingElement.type === "checkbox" || matchingElement.type === "radio") {
+                    if (matchingElement.className === this.ValidationInputValidCssClassName) {
+                        continue;
+                    }
+                }
+            }
+
+            // With required multiple inputs, such as a checkbox list, we'll have one message per input.
+            // It's one from the inputs that's required, not all, so we should only have one message displayed.
+            if (renderedMessages.indexOf(this.summary[key]) > -1) {
+                continue;
+            }
+
             let li = document.createElement('li');
             li.innerHTML = this.summary[key];
             ul.appendChild(li);
+            renderedMessages.push(this.summary[key]);
         }
         return ul;
     }
@@ -978,6 +998,7 @@ export class ValidationService {
             return;
         }
 
+        // Prevents wasteful re-rendering of summary list element with identical items!
         // Using JSON.stringify for quick and painless deep compare of simple KVP. You need to sort the keys first, tho...
         let shadow = JSON.stringify(this.summary, Object.keys(this.summary).sort());
         if (shadow === this.renderedSummaryJSON) {
@@ -990,8 +1011,15 @@ export class ValidationService {
 
         for (let i = 0; i < summaryElements.length; i++) {
             let e = summaryElements[i];
-            e.innerHTML = '';
-            if (ul) {
+
+            // Remove existing list elements, but keep the summary's message.
+            let listElements = e.querySelectorAll("ul");
+            for (let j = 0; j < listElements.length; j++) {
+                listElements[j].remove();
+            }
+
+            // Style the summary element as valid/invalid depending on whether there are any messages to display.
+            if (ul && ul.hasChildNodes()) {
                 this.swapClasses(e,
                     this.ValidationSummaryCssClassName,
                     this.ValidationSummaryValidCssClassName)
@@ -1033,11 +1061,12 @@ export class ValidationService {
                 this.swapClasses(inputs[i],
                     this.ValidationInputCssClassName,
                     this.ValidationInputValidCssClassName);
+
+                let uid = this.getElementUID(inputs[i]);
+                this.summary[uid] = message;
             }
         }
 
-        let uid = this.getElementUID(input);
-        this.summary[uid] = message;
         this.renderSummary();
     }
 
@@ -1067,11 +1096,12 @@ export class ValidationService {
                 this.swapClasses(inputs[i],
                     this.ValidationInputValidCssClassName,
                     this.ValidationInputCssClassName);
+
+                let uid = this.getElementUID(inputs[i]);
+                delete this.summary[uid];
             }
         }
 
-        let uid = this.getElementUID(input);
-        delete this.summary[uid];
         this.renderSummary();
     }