Reset error styling on the terms checkboxes (#491) - #1086
Open
gangster wants to merge 1 commit into
Open
Conversation
Field errors are applied to the input's closest '.form-group' or
'.checkbox' container, but the reset at the top of the save handler
only cleared '.form-group'. The two terms checkboxes on the new-ride
form live in '<div class="checkbox">', so has-error was added to them
and never removed.
The adjacent resets are selector-agnostic ($('[aria-invalid="true"]')
and $('.help-block')), which is why only the red styling persisted:
a corrected checkbox stayed red with its explanatory message gone.
The stale class also fed the accordion logic, which keys off
$('.has-error') to decide which panels to show, pinning the Terms
panel open on every resubmit.
Only the new-ride form is affected; the edit form has no Terms panel.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #491.
Affects the new-ride submission form at
/addevent/. The edit form for an existing ride has no Terms panel, so it isn't impacted.The problem
The error-styling reset and the error-styling application disagree about which containers they target.
Errors are applied to either container type:
But the reset at the top of the save handler only cleared one of them:
The two terms checkboxes live in
<div class="checkbox">(edit.html:469,:476), not.form-group, sohas-errorwas added to them and never removed — there is no other code path that clears it.The neighbouring resets are selector-agnostic —
$('[aria-invalid="true"]')and$('.help-block')— which is exactly why the reported symptom looks the way it does: the aria state and the help text both clear correctly, and only the red styling sticks.The fix
One selector, bringing the reset in line with the containers the error path already uses:
Two things beyond what the issue describes
1. The user got red with no explanation. Because
$('.help-block').remove()is selector-agnostic, the message under the checkbox was being cleared whilehas-errorstayed. So a corrected checkbox rendered red with no reason given, while the genuinely-broken fields above it were red with their "Organizer missing" / "Email missing" text. The one control the user had actually fixed was the one carrying unexplained red.2. It also pinned the Terms panel open. The accordion logic keys off the same class:
The stale
has-errorkept the Terms accordion inerrGroups, so it was held open on every resubmit even with nothing wrong in it. After this change the panel state is correct — Terms collapses once its checkboxes are ticked, and Contact Info stays open while it still has real errors.Verification
Ran the issue's repro steps in Chrome against a local dev build, on the unmodified file and again on the patched one.
.form-group.has-error.checkbox.has-erroraria-invalidfalsefalsergb(169, 68, 66)rgb(64, 64, 64)The 6 legitimate
.form-grouperrors surviving in both columns is the control — corrected checkboxes reset without over-clearing errors that are still real.Regression checks
/addevent/event-submitted. Backend created the event unpublished with the confirmation link logged, as expected.npm testis 53/53, unchanged. (Backend-only suite, so it doesn't cover this file — noted for completeness rather than as evidence.)To reproduce / review
npm run dev, then openhttp://localhost:3080/addevent/(the new-ride form, not an edit link)Before: both checkbox labels stay red, with no message, and the Terms panel stays open.
After: they return to normal, the Terms panel collapses, and the remaining real errors are untouched.
Note for whoever picks this up next
The issue links to
/site/themes/s2b_hugo_theme/static/js/cal/addevent.js#L186and#L129. That file now lives underassets/js/cal/(Hugo asset pipeline) and those line numbers no longer point at the relevant code.