Skip to content

Commit f0d8982

Browse files
committed
[Bug Fix] Address cubic review: sanitize prefilled value, fix docs typo
- connect() now runs the same filter+truncate as onInput before the first paint, so a server-rendered value: that exceeds length or contains characters outside pattern can't hide behind the slots and get submitted as-is. - Fix "length:." double-punctuation typo in the Four digits example. - Note in the plan doc that its Task 4 controller snapshot predates later bug-fix commits (normalizeSelection, arrow-key range selection) — cubic flagged both as "missing" because it reviewed the plan, not gem/lib/ruby_ui/input_otp/input_otp_controller.js where they already exist.
1 parent b33c865 commit f0d8982

6 files changed

Lines changed: 29 additions & 7 deletions

File tree

design/plans/2026-06-30-input-otp-implementation.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,8 @@ git commit -m "[Feature] Add InputOtpSlot"
374374

375375
- [ ] **Step 1: Write the implementation**
376376

377+
> **Superseded:** the draft below is what Task 4 originally shipped. Manual browser testing (Task 7) and PR review surfaced real bugs in it — collapsed-caret arrow navigation, no replace-and-advance after editing a full value, no sanitization of a prefilled `value:` on connect — all fixed in follow-up commits. Treat `gem/lib/ruby_ui/input_otp/input_otp_controller.js` as the source of truth, not this snapshot.
378+
377379
Create `gem/lib/ruby_ui/input_otp/input_otp_controller.js`:
378380

379381
```js

docs/app/javascript/controllers/ruby_ui/input_otp_controller.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ export default class extends Controller {
66
static values = { length: Number, charClass: String }
77

88
connect() {
9+
// A server-rendered value (prefilled from a previous submission, a
10+
// validation error, etc.) may exceed length or contain characters that
11+
// fail pattern. Sanitize it up front so the hidden slots never mask
12+
// an invalid/oversized value that would otherwise still get submitted.
13+
this.sanitizeValue()
914
this.paint()
1015
this.boundOnSelectionChange = this.onSelectionChange.bind(this)
1116
document.addEventListener("selectionchange", this.boundOnSelectionChange)
@@ -16,8 +21,8 @@ export default class extends Controller {
1621
}
1722

1823
onInput() {
19-
const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)
20-
if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered
24+
this.sanitizeValue()
25+
const filtered = this.inputTarget.value
2126

2227
this.normalizeSelection()
2328
this.paint()
@@ -98,6 +103,11 @@ export default class extends Controller {
98103
return raw.split("").filter((char) => re.test(char)).join("")
99104
}
100105

106+
sanitizeValue() {
107+
const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)
108+
if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered
109+
}
110+
101111
paint() {
102112
const value = this.inputTarget.value
103113
const isFocused = document.activeElement === this.inputTarget

docs/app/views/docs/input_otp.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def view_template
7070

7171
Heading(level: 2) { "Four digits" }
7272

73-
Text { "A common pattern for PIN codes — just pass a shorter length:." }
73+
Text { "A common pattern for PIN codes — just pass a shorter length." }
7474

7575
render Docs::VisualCodeExample.new(title: "Example", context: self) do
7676
<<~RUBY

gem/lib/ruby_ui/input_otp/input_otp_controller.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ export default class extends Controller {
66
static values = { length: Number, charClass: String }
77

88
connect() {
9+
// A server-rendered value (prefilled from a previous submission, a
10+
// validation error, etc.) may exceed length or contain characters that
11+
// fail pattern. Sanitize it up front so the hidden slots never mask
12+
// an invalid/oversized value that would otherwise still get submitted.
13+
this.sanitizeValue()
914
this.paint()
1015
this.boundOnSelectionChange = this.onSelectionChange.bind(this)
1116
document.addEventListener("selectionchange", this.boundOnSelectionChange)
@@ -16,8 +21,8 @@ export default class extends Controller {
1621
}
1722

1823
onInput() {
19-
const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)
20-
if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered
24+
this.sanitizeValue()
25+
const filtered = this.inputTarget.value
2126

2227
this.normalizeSelection()
2328
this.paint()
@@ -98,6 +103,11 @@ export default class extends Controller {
98103
return raw.split("").filter((char) => re.test(char)).join("")
99104
}
100105

106+
sanitizeValue() {
107+
const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)
108+
if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered
109+
}
110+
101111
paint() {
102112
const value = this.inputTarget.value
103113
const isFocused = document.activeElement === this.inputTarget

gem/lib/ruby_ui/input_otp/input_otp_docs.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def view_template
7070

7171
Heading(level: 2) { "Four digits" }
7272

73-
Text { "A common pattern for PIN codes — just pass a shorter length:." }
73+
Text { "A common pattern for PIN codes — just pass a shorter length." }
7474

7575
render Docs::VisualCodeExample.new(title: "Example", context: self) do
7676
<<~RUBY

mcp/data/registry.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1611,7 +1611,7 @@
16111611
},
16121612
{
16131613
"path": "input_otp_controller.js",
1614-
"content": "import { Controller } from \"@hotwired/stimulus\"\n\n// Connects to data-controller=\"ruby-ui--input-otp\"\nexport default class extends Controller {\n static targets = [\"input\", \"slot\"]\n static values = { length: Number, charClass: String }\n\n connect() {\n this.paint()\n this.boundOnSelectionChange = this.onSelectionChange.bind(this)\n document.addEventListener(\"selectionchange\", this.boundOnSelectionChange)\n }\n\n disconnect() {\n document.removeEventListener(\"selectionchange\", this.boundOnSelectionChange)\n }\n\n onInput() {\n const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)\n if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered\n\n this.normalizeSelection()\n this.paint()\n this.dispatch(\"input\", { detail: { value: filtered } })\n if (filtered.length === this.lengthValue) {\n this.dispatch(\"complete\", { detail: { value: filtered } })\n }\n }\n\n onFocus() {\n const end = this.inputTarget.value.length\n const start = Math.min(end, this.lengthValue - 1)\n this.inputTarget.setSelectionRange(start, end)\n this.paint()\n }\n\n onBlur() {\n this.paint()\n }\n\n onKeydown(event) {\n const moves = { ArrowLeft: -1, ArrowUp: -1, ArrowRight: 1, ArrowDown: 1 }\n if (!(event.key in moves)) return\n\n event.preventDefault()\n const current = this.inputTarget.selectionStart ?? 0\n const next = Math.min(Math.max(current + moves[event.key], 0), this.lengthValue - 1)\n const hasChar = next < this.inputTarget.value.length\n this.inputTarget.setSelectionRange(next, hasChar ? next + 1 : next)\n this.paint()\n }\n\n onPaste(event) {\n event.preventDefault()\n const pasted = this.filter(event.clipboardData.getData(\"text/plain\"))\n if (!pasted) return\n\n const start = this.inputTarget.selectionStart ?? 0\n const end = this.inputTarget.selectionEnd ?? start\n const current = this.inputTarget.value\n const merged = (current.slice(0, start) + pasted + current.slice(end)).slice(0, this.lengthValue)\n\n this.inputTarget.value = merged\n const caret = Math.min(merged.length, this.lengthValue - 1)\n this.inputTarget.setSelectionRange(caret, merged.length)\n\n this.paint()\n this.dispatch(\"input\", { detail: { value: merged } })\n if (merged.length === this.lengthValue) this.dispatch(\"complete\", { detail: { value: merged } })\n }\n\n onSelectionChange() {\n if (document.activeElement !== this.inputTarget) return\n this.paint()\n }\n\n // After typing, replacing, or deleting, the browser leaves a collapsed\n // caret. If it landed on a slot that already has a character (not the\n // true insert-mode end of the value), re-select that character as a\n // 1-char range so the next keystroke replaces it instead of being\n // silently dropped by the native maxlength/no-selection behavior.\n normalizeSelection() {\n const input = this.inputTarget\n const value = input.value\n const s = input.selectionStart\n const e = input.selectionEnd\n if (s === null || e === null || s !== e) return\n\n const isInsertMode = s === value.length && value.length < this.lengthValue\n if (isInsertMode) return\n\n const index = Math.min(s, this.lengthValue - 1)\n input.setSelectionRange(index, index < value.length ? index + 1 : index)\n }\n\n filter(raw) {\n const re = new RegExp(this.charClassValue)\n return raw.split(\"\").filter((char) => re.test(char)).join(\"\")\n }\n\n paint() {\n const value = this.inputTarget.value\n const isFocused = document.activeElement === this.inputTarget\n const start = this.inputTarget.selectionStart ?? value.length\n const end = this.inputTarget.selectionEnd ?? value.length\n const activeIndex = Math.min(start, this.lengthValue - 1)\n\n this.slotTargets.forEach((slot) => {\n const index = Number(slot.dataset.index)\n const char = value[index] ?? \"\"\n const isActive = isFocused && ((start === end && index === activeIndex) || (index >= start && index < end))\n\n slot.textContent = char\n slot.dataset.active = isActive ? \"true\" : \"false\"\n slot.dataset.caret = isActive && char === \"\" ? \"true\" : \"false\"\n })\n }\n}\n"
1614+
"content": "import { Controller } from \"@hotwired/stimulus\"\n\n// Connects to data-controller=\"ruby-ui--input-otp\"\nexport default class extends Controller {\n static targets = [\"input\", \"slot\"]\n static values = { length: Number, charClass: String }\n\n connect() {\n // A server-rendered value (prefilled from a previous submission, a\n // validation error, etc.) may exceed length or contain characters that\n // fail pattern. Sanitize it up front so the hidden slots never mask\n // an invalid/oversized value that would otherwise still get submitted.\n this.sanitizeValue()\n this.paint()\n this.boundOnSelectionChange = this.onSelectionChange.bind(this)\n document.addEventListener(\"selectionchange\", this.boundOnSelectionChange)\n }\n\n disconnect() {\n document.removeEventListener(\"selectionchange\", this.boundOnSelectionChange)\n }\n\n onInput() {\n this.sanitizeValue()\n const filtered = this.inputTarget.value\n\n this.normalizeSelection()\n this.paint()\n this.dispatch(\"input\", { detail: { value: filtered } })\n if (filtered.length === this.lengthValue) {\n this.dispatch(\"complete\", { detail: { value: filtered } })\n }\n }\n\n onFocus() {\n const end = this.inputTarget.value.length\n const start = Math.min(end, this.lengthValue - 1)\n this.inputTarget.setSelectionRange(start, end)\n this.paint()\n }\n\n onBlur() {\n this.paint()\n }\n\n onKeydown(event) {\n const moves = { ArrowLeft: -1, ArrowUp: -1, ArrowRight: 1, ArrowDown: 1 }\n if (!(event.key in moves)) return\n\n event.preventDefault()\n const current = this.inputTarget.selectionStart ?? 0\n const next = Math.min(Math.max(current + moves[event.key], 0), this.lengthValue - 1)\n const hasChar = next < this.inputTarget.value.length\n this.inputTarget.setSelectionRange(next, hasChar ? next + 1 : next)\n this.paint()\n }\n\n onPaste(event) {\n event.preventDefault()\n const pasted = this.filter(event.clipboardData.getData(\"text/plain\"))\n if (!pasted) return\n\n const start = this.inputTarget.selectionStart ?? 0\n const end = this.inputTarget.selectionEnd ?? start\n const current = this.inputTarget.value\n const merged = (current.slice(0, start) + pasted + current.slice(end)).slice(0, this.lengthValue)\n\n this.inputTarget.value = merged\n const caret = Math.min(merged.length, this.lengthValue - 1)\n this.inputTarget.setSelectionRange(caret, merged.length)\n\n this.paint()\n this.dispatch(\"input\", { detail: { value: merged } })\n if (merged.length === this.lengthValue) this.dispatch(\"complete\", { detail: { value: merged } })\n }\n\n onSelectionChange() {\n if (document.activeElement !== this.inputTarget) return\n this.paint()\n }\n\n // After typing, replacing, or deleting, the browser leaves a collapsed\n // caret. If it landed on a slot that already has a character (not the\n // true insert-mode end of the value), re-select that character as a\n // 1-char range so the next keystroke replaces it instead of being\n // silently dropped by the native maxlength/no-selection behavior.\n normalizeSelection() {\n const input = this.inputTarget\n const value = input.value\n const s = input.selectionStart\n const e = input.selectionEnd\n if (s === null || e === null || s !== e) return\n\n const isInsertMode = s === value.length && value.length < this.lengthValue\n if (isInsertMode) return\n\n const index = Math.min(s, this.lengthValue - 1)\n input.setSelectionRange(index, index < value.length ? index + 1 : index)\n }\n\n filter(raw) {\n const re = new RegExp(this.charClassValue)\n return raw.split(\"\").filter((char) => re.test(char)).join(\"\")\n }\n\n sanitizeValue() {\n const filtered = this.filter(this.inputTarget.value).slice(0, this.lengthValue)\n if (filtered !== this.inputTarget.value) this.inputTarget.value = filtered\n }\n\n paint() {\n const value = this.inputTarget.value\n const isFocused = document.activeElement === this.inputTarget\n const start = this.inputTarget.selectionStart ?? value.length\n const end = this.inputTarget.selectionEnd ?? value.length\n const activeIndex = Math.min(start, this.lengthValue - 1)\n\n this.slotTargets.forEach((slot) => {\n const index = Number(slot.dataset.index)\n const char = value[index] ?? \"\"\n const isActive = isFocused && ((start === end && index === activeIndex) || (index >= start && index < end))\n\n slot.textContent = char\n slot.dataset.active = isActive ? \"true\" : \"false\"\n slot.dataset.caret = isActive && char === \"\" ? \"true\" : \"false\"\n })\n }\n}\n"
16151615
},
16161616
{
16171617
"path": "input_otp_group.rb",

0 commit comments

Comments
 (0)