Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove duplicate validation summary entries #57

Merged
merged 2 commits into from
Jul 24, 2023
Merged

Remove duplicate validation summary entries #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7cc4f24
Fixed the rendering of duplicate messages within the validation summary.
AndyButland Jul 24, 2023
7db2a86
Removed validation summary added on index page.
AndyButland Jul 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion Pages/Demos/Checkboxes.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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>
Expand Down
6 changes: 6 additions & 0 deletions Pages/Demos/Checkboxes.cshtml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
2 changes: 1 addition & 1 deletion Pages/Index.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 4 additions & 0 deletions dist/aspnet-validation.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@
color: red;
font-style: italic;
}

.validation-summary-valid span {
display: none;
}
35 changes: 29 additions & 6 deletions dist/aspnet-validation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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) {
Expand All @@ -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));
}
Expand Down Expand Up @@ -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();
};
/**
Expand All @@ -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();
};
/**
Expand Down
2 changes: 1 addition & 1 deletion dist/aspnet-validation.min.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/aspnet-validation.min.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/aspnet-validation.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@
color: red;
font-style: italic;
}

.validation-summary-valid span {
display: none;
}
42 changes: 36 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand Down