Skip to content

Commit 52791d9

Browse files
committed
Address review: no-JS fallback and NaN guard
- Scope the hiding rule to a class the script sets on load, so a failed or disabled script leaves the fields visible in their rendered position instead of hidden permanently - Guard snapToStep against non-finite parses, which previously could write NaN:NaN back into the input
1 parent eb643a1 commit 52791d9

2 files changed

Lines changed: 16 additions & 4 deletions

File tree

assets/javascripts/issue_datetime.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,24 @@
88
(function () {
99
'use strict';
1010

11+
// Marks that this script actually loaded. The stylesheet only hides the
12+
// detached container when this class is present, so a failed or disabled
13+
// script leaves the fields visible where they were rendered instead of
14+
// hiding them forever.
15+
document.documentElement.classList.add('issue-datetime-js');
16+
1117
function snapToStep(input) {
1218
var step = parseInt(input.getAttribute('step'), 10);
1319
if (!step || !input.value) return;
1420

1521
var parts = input.value.split(':');
1622
if (parts.length < 2) return;
1723

18-
var minutes = parseInt(parts[0], 10) * 60 + parseInt(parts[1], 10);
24+
var hours = parseInt(parts[0], 10);
25+
var mins = parseInt(parts[1], 10);
26+
if (!isFinite(hours) || !isFinite(mins)) return;
27+
28+
var minutes = hours * 60 + mins;
1929
var stepMinutes = step / 60;
2030
if (!stepMinutes) return;
2131

assets/stylesheets/issue_datetime.css

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
/* Hide the detached fields until issue_datetime.js has placed them, so they
2-
do not flash at the bottom of the form on load. */
3-
#issue-datetime-fields.issue-datetime-pending {
1+
/* Hide the detached fields until issue_datetime.js has placed them, so they do
2+
not flash at the bottom of the form on load. Scoped to the class that script
3+
sets on load: if the script is missing or disabled, the fields stay visible
4+
and usable in their rendered position rather than hidden for good. */
5+
.issue-datetime-js #issue-datetime-fields.issue-datetime-pending {
46
display: none;
57
}
68

0 commit comments

Comments
 (0)