Conversation
📝 WalkthroughWalkthroughAdds a delimiter feature to the PinInput component. Introduces an optional Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/runtime/components/PinInput.vue (1)
152-156: Consider exposing delimiter styling through the themeuislots.The hardcoded
<span class="text-dimmed">•</span>and wrapper<span>aren't themeable viaui.delimiterlike the rest of the component (e.g.,ui.base,ui.root). For consistency with other components and to let users override spacing/color without resorting to the slot, consider adding adelimiterslot entry to the theme and applyingui.delimiter({ class: uiProp?.delimiter })here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/PinInput.vue` around lines 152 - 156, The delimiter markup in PinInput.vue currently uses a hardcoded <span class="text-dimmed">•</span>; change this to use the theme `ui.delimiter` slot so users can style it: when rendering the delimiter (where `delimiterPositions?.includes((index as number) + 1)`), apply the theme helper like `ui.delimiter({ class: uiProp?.delimiter })` to the wrapper/element instead of the fixed "text-dimmed" class and keep the named slot `delimiter` as fallback; also add a corresponding `ui.delimiter` entry to the theme config so consumers can override spacing/color consistently with `ui.base`/`ui.root`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/2.components/pin-input.md`:
- Line 173: The "Delimiter" heading is for a new feature and needs the Soon
badge; update the heading "### Delimiter" to include the badge token
:badge{label="Soon" class="align-text-top"} immediately after the heading text
so it reads like the other new-feature headings in this doc (ensure you modify
the heading line that contains "### Delimiter").
In `@src/runtime/components/PinInput.vue`:
- Around line 152-156: The delimiter check in PinInput.vue uses
delimiterPositions?.includes((index as number) + 1) which can match a position
equal to length and render a trailing delimiter; update the condition to also
ensure the computed position is less than the total length (e.g., only render
when (index + 1) < length and delimiterPositions includes that position). Modify
the span's v-if to combine the includes check with a guard against positions >=
length so delimiters are only inserted between inputs (referencing
delimiterPositions, index, and length in the PinInput component).
---
Nitpick comments:
In `@src/runtime/components/PinInput.vue`:
- Around line 152-156: The delimiter markup in PinInput.vue currently uses a
hardcoded <span class="text-dimmed">•</span>; change this to use the theme
`ui.delimiter` slot so users can style it: when rendering the delimiter (where
`delimiterPositions?.includes((index as number) + 1)`), apply the theme helper
like `ui.delimiter({ class: uiProp?.delimiter })` to the wrapper/element instead
of the fixed "text-dimmed" class and keep the named slot `delimiter` as
fallback; also add a corresponding `ui.delimiter` entry to the theme config so
consumers can override spacing/color consistently with `ui.base`/`ui.root`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc64eb2f-181d-46f7-9495-08a6538a6d46
⛔ Files ignored due to path filters (2)
test/components/__snapshots__/PinInput-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/PinInput.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
docs/content/docs/2.components/pin-input.mdplaygrounds/nuxt/app/pages/components/pin-input.vuesrc/runtime/components/PinInput.vue
commit: |
…dd soon badge in docs
al1maher
changed the title
al1maher
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/runtime/components/PinInput.vue (1)
118-120: Trailing-delimiter guard looks good; drop redundantas numbercasts.The
(index as number) + 1 < looseToNumber(props.length)guard correctly prevents a dangling delimiter after the last input. Minor nit: sinceindexis already typed asnumberin the function signature, bothindex as numbercasts are no-ops and can be simplified.♻️ Proposed simplification
function shouldInsertDelimiter(index: number) { - return props.delimiterPositions?.includes((index as number) + 1) && (index as number) + 1 < looseToNumber(props.length) + return props.delimiterPositions?.includes(index + 1) && index + 1 < looseToNumber(props.length) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/PinInput.vue` around lines 118 - 120, The function shouldInsertDelimiter currently contains redundant "as number" casts on the already-typed parameter index; simplify by removing those casts and use index + 1 directly in both places while keeping the existing logic and guards (props.delimiterPositions?.includes(index + 1) && index + 1 < looseToNumber(props.length)); update references to props.delimiterPositions and props.length only (no other logic changes).docs/content/docs/2.components/pin-input.md (1)
211-215: Verify the blank line underdelimiter: |doesn't leak into the rendered slot.The literal block scalar preserves the empty line between
|and<span …>, which may render as a leading empty text node inside the delimiter slot (and potentially a stray line break in the demo). If the playground component-code loader trims it, this is fine — otherwise remove the blank line.♻️ Proposed tidy-up
slots: delimiter: | - <span class="text-primary font-bold">-</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/2.components/pin-input.md` around lines 211 - 215, The literal block under the YAML frontmatter for the delimiter slot contains an extra blank line after "delimiter: |" which can produce an unwanted leading empty text node; remove that blank line so the content starts immediately after "delimiter: |" (i.e., ensure the slot's literal block begins with "<span class=\"text-primary font-bold\">-</span>" on the next line) to prevent a stray empty node in the delimiter slot rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/components/PinInput.vue`:
- Around line 42-46: The component's paste/autofill isn't sanitizing delimiter
characters so pasted text like "123-456" can populate delimiter slots; add
normalization that strips any characters at delimiterPositions boundaries before
distributing into inputs: implement an onPaste handler on the root/input
elements that reads clipboard data, removes characters at indices corresponding
to delimiterPositions (or strips known delimiter chars), and then emits the
cleaned value via update:model-value (or calls the existing value-distribution
logic); also apply the same normalization to any external updates handled by the
update:model-value path so programmatic Autofill/paste-like updates are
normalized; reference PinInput.vue props/methods: delimiterPositions,
update:model-value event, and the input distribution logic to locate where to
integrate the sanitization.
---
Nitpick comments:
In `@docs/content/docs/2.components/pin-input.md`:
- Around line 211-215: The literal block under the YAML frontmatter for the
delimiter slot contains an extra blank line after "delimiter: |" which can
produce an unwanted leading empty text node; remove that blank line so the
content starts immediately after "delimiter: |" (i.e., ensure the slot's literal
block begins with "<span class=\"text-primary font-bold\">-</span>" on the next
line) to prevent a stray empty node in the delimiter slot rendering.
In `@src/runtime/components/PinInput.vue`:
- Around line 118-120: The function shouldInsertDelimiter currently contains
redundant "as number" casts on the already-typed parameter index; simplify by
removing those casts and use index + 1 directly in both places while keeping the
existing logic and guards (props.delimiterPositions?.includes(index + 1) &&
index + 1 < looseToNumber(props.length)); update references to
props.delimiterPositions and props.length only (no other logic changes).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7f4c71d-731c-4320-99bf-0aa111fef563
📒 Files selected for processing (2)
docs/content/docs/2.components/pin-input.mdsrc/runtime/components/PinInput.vue
| /** | ||
| * Position(s) after which to insert a delimiter. | ||
| * @example [3, 6] // Insert delimiters after 3rd and 6th inputs | ||
| */ | ||
| delimiterPositions?: number[] |
There was a problem hiding this comment.
Consider paste/autofill sanitization to fully address issue #6389.
The linked issue calls out robust paste/autofill handling — e.g., pasting "123-456" or "123 456" into a PIN that displays delimiters at position 3 should strip the delimiter characters and fill ["1","2","3","4","5","6"]. The current implementation is display-only (delimiter is a sibling <span>, so it never enters the inputs), which means:
- With
type="number", reka-ui already filters non-numeric input on paste — fine. - With
type="text", a paste of"123-456"will distribute-into the 4th slot rather than being treated as a delimiter and skipped.
If that's intentionally out of scope for this PR, consider calling it out as a known limitation in the docs; otherwise, an @paste/update:model-value normalization step that strips characters appearing at delimiterPositions boundaries would close the gap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/components/PinInput.vue` around lines 42 - 46, The component's
paste/autofill isn't sanitizing delimiter characters so pasted text like
"123-456" can populate delimiter slots; add normalization that strips any
characters at delimiterPositions boundaries before distributing into inputs:
implement an onPaste handler on the root/input elements that reads clipboard
data, removes characters at indices corresponding to delimiterPositions (or
strips known delimiter chars), and then emits the cleaned value via
update:model-value (or calls the existing value-distribution logic); also apply
the same normalization to any external updates handled by the update:model-value
path so programmatic Autofill/paste-like updates are normalized; reference
PinInput.vue props/methods: delimiterPositions, update:model-value event, and
the input distribution logic to locate where to integrate the sanitization.
1 participant
Allow splitting up input to improve readability or consistency - depending on how a brand prefers to present the code.
🔗 Linked issue
Resolves #6389
❓ Type of change
📚 Description
📝 Checklist