feat(keyboard-full): add inputType prop for password masking#1048
feat(keyboard-full): add inputType prop for password masking#1048ludvigovrevik wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Forward an input type to the embedded obc-text-input-field via a new `inputType` property (defaults to `text`). Setting `inputType="password"` masks the entered value and surfaces the field's built-in show/hide toggle. Key handling is unchanged: value-change and done-click still emit the real, unmasked string. Named `inputType` rather than `type` because `type` already selects the visual variant (floating/flat), mirroring the existing `inputSize` prop. Add a Password story demonstrating the masked input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a design spec and implementation for an ChangesPassword masking input type support
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant ObcKeyboardFull
participant ObcTextInputField
Consumer->>ObcKeyboardFull: set inputType="password"
ObcKeyboardFull->>ObcTextInputField: .type=${this.inputType}
ObcTextInputField-->>ObcKeyboardFull: masked display, value-change event
ObcKeyboardFull-->>Consumer: value-change/done-click with real value
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
The demo can be viewed at https://openbridge-next-demo--1048-feat-keyboard-full-add-inpu-v003hl7m.web.app |
|
The storybook can be viewed at https://openbridge-next-storybook--1048-feat-keyboard-full-add-ywzv3eo2.web.app |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/superpowers/specs/2026-07-07-keyboard-full-input-type-design.md (2)
63-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit test coverage for the forwarding contract, not just a visual snapshot.
The "Testing" section only lists a visual snapshot for the
Passwordstory plus typecheck/lint. Consider adding a unit test inkeyboard-full.spec.tsasserting thatinputTypeis actually forwarded to the embeddedobc-text-input-field's.type(e.g., setinputType = HTMLInputTypeAttribute.Passwordand assert the renderedobc-text-input-field'stypeproperty). A visual snapshot alone won't catch a regression in the binding itself.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-07-keyboard-full-input-type-design.md` around lines 63 - 67, Add unit test coverage for the forwarding contract in keyboard-full.spec.ts, not just the Password visual snapshot. Create a test that sets inputType to HTMLInputTypeAttribute.Password and verifies the embedded obc-text-input-field receives the same value through its .type property, so the binding is exercised directly. Keep the existing snapshot/typecheck/lint checks, but make sure the new spec explicitly asserts the inputType-to-type forwarding behavior.
69-73: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider documenting the
date/timelimitation explicitly.The doc calls out that email/tel/url "become available for free" but doesn't mention
date/time, both also part ofHTMLInputTypeAttribute. Since this keyboard drives the embedded field's value via property assignment (this.value) rather than simulated native keystrokes, arbitrary characters typed fordate/timeinputs generally won't conform to the strict format native<input type="date"|"time">requires for its displayed value — so the field may visually appear empty/unchanged even thoughvalue-change/done-clickstill carry the typed string. Worth an explicit note (or scoping the exposed options) so consumers don't silently hit this.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-07-keyboard-full-input-type-design.md` around lines 69 - 73, Document the `date`/`time` limitation in the keyboard full input type spec, since `obc-keyboard-full` exposes `HTMLInputTypeAttribute` values but updates the embedded field through `this.value` assignment rather than native keystroke simulation. In the spec/story/docs section that currently mentions email/tel/url, add an explicit note that `date` and `time` inputs may not visually update or may appear unchanged unless the typed string matches the native format, even though `value-change` and `done-click` still emit the raw string. If needed, scope the exposed options in the `KeyboardFull`/`HTMLInputTypeAttribute` docs so consumers understand the limitation.packages/openbridge-webcomponents/src/components/keyboard-full/keyboard-full.ts (1)
708-708: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify behavior for
date/timeinputType values.Forwarding
this.inputTypehere is correct fortext/password/tel/url/search, since those accept arbitrary typed strings and (for text/search/url/tel/password) still support cursor tracking viaselectionStart. Fordate/time,selectionStartreads asnullon those types (confirmed: per WHATWG/MDN,selectionStartonly applies to text/search/url/tel/password; other types returnnullon read), so the existing null-guards inonKeyPress/onCursorLeft/onCursorRightetc. degrade gracefully to append/no-op rather than throwing — no crash. However, since<input type="date"|"time">requires a strict conforming value format, free-text key presses routed throughthis.value→.value=${this.value}likely won't render visibly in the embedded field for those two types, even thoughvalue-change/done-clickstill emit the raw string. Since the StorybookinputTypeselect control (stories.ts:93-101) exposes all 8 enum values includingdate/timewith no caveat, a consumer could pick a non-functional-looking configuration.Consider either restricting the exposed/supported values to the ones that behave as expected (text/password/email/tel/url/search), or documenting the date/time caveat in the property JSDoc.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openbridge-webcomponents/src/components/keyboard-full/keyboard-full.ts` at line 708, The keyboard-full component currently forwards this.inputType to the embedded input, but date and time do not behave like the text-like types supported by the keyboard. Update the keyboard-full behavior around the input binding and related inputType handling so the supported values are limited to the modes that accept typed strings, or clearly document in the inputType property JSDoc and Storybook controls that date/time are a special case with non-visual/raw-value behavior. Use the inputType setter/binding and the onKeyPress/onCursorLeft/onCursorRight paths to keep the supported contract consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/superpowers/specs/2026-07-07-keyboard-full-input-type-design.md`:
- Around line 63-67: Add unit test coverage for the forwarding contract in
keyboard-full.spec.ts, not just the Password visual snapshot. Create a test that
sets inputType to HTMLInputTypeAttribute.Password and verifies the embedded
obc-text-input-field receives the same value through its .type property, so the
binding is exercised directly. Keep the existing snapshot/typecheck/lint checks,
but make sure the new spec explicitly asserts the inputType-to-type forwarding
behavior.
- Around line 69-73: Document the `date`/`time` limitation in the keyboard full
input type spec, since `obc-keyboard-full` exposes `HTMLInputTypeAttribute`
values but updates the embedded field through `this.value` assignment rather
than native keystroke simulation. In the spec/story/docs section that currently
mentions email/tel/url, add an explicit note that `date` and `time` inputs may
not visually update or may appear unchanged unless the typed string matches the
native format, even though `value-change` and `done-click` still emit the raw
string. If needed, scope the exposed options in the
`KeyboardFull`/`HTMLInputTypeAttribute` docs so consumers understand the
limitation.
In
`@packages/openbridge-webcomponents/src/components/keyboard-full/keyboard-full.ts`:
- Line 708: The keyboard-full component currently forwards this.inputType to the
embedded input, but date and time do not behave like the text-like types
supported by the keyboard. Update the keyboard-full behavior around the input
binding and related inputType handling so the supported values are limited to
the modes that accept typed strings, or clearly document in the inputType
property JSDoc and Storybook controls that date/time are a special case with
non-visual/raw-value behavior. Use the inputType setter/binding and the
onKeyPress/onCursorLeft/onCursorRight paths to keep the supported contract
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff2632f2-b5ee-4b33-8d8a-480240623c49
⛔ Files ignored due to path filters (1)
packages/openbridge-webcomponents/__vis__/linux/__baselines__/components/keyboard-full/keyboard-full.stories.ts/password-auto.pngis excluded by!**/*.png
📒 Files selected for processing (3)
docs/superpowers/specs/2026-07-07-keyboard-full-input-type-design.mdpackages/openbridge-webcomponents/src/components/keyboard-full/keyboard-full.stories.tspackages/openbridge-webcomponents/src/components/keyboard-full/keyboard-full.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
A user asked to mask password input on the virtual keyboard.
obc-text-input-fieldalready supports masking (type="password"renders a masked input plus a show/hide toggle), butobc-keyboard-fullnever forwarded an input type to its embedded field — so it always rendered as plain text.Change
Add an
inputTypeproperty toobc-keyboard-fulland forward it to the embeddedobc-text-input-field:HTMLInputTypeAttributeenum (text|password|email|tel|url| ...), defaulting totext— no behavior change for existing consumers.inputType(nottype) becausetypealready selects the visual variant (floating/flat); this mirrors the existinginputSizeprop.value-change/done-clickstill emit the real (unmasked) string. Only the field's display is masked. The show/hide toggle is rendered by the input field itself.Story / baseline
Added a
Passwordstory (maskedsecret123, number row on) and its Linux visual baseline. Verified by eye: input shows dots + the eye toggle.Baseline hygiene
Only the single new
password-auto.pngbaseline is included. The--updaterun also touched ~9 unrelated baselines (known local-vs-CI renderer noise); those were discarded and are not in this PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
inputTypeoption to the full keyboard component, with a default oftext.Behavior