Skip to content

Add an in-field reveal toggle for secure inputs - #61

Open
CodyPChristian wants to merge 1 commit into
NativePHP:mainfrom
CodyPChristian:feat/secure-input-reveal-toggle
Open

Add an in-field reveal toggle for secure inputs#61
CodyPChristian wants to merge 1 commit into
NativePHP:mainfrom
CodyPChristian:feat/secure-input-reveal-toggle

Conversation

@CodyPChristian

Copy link
Copy Markdown
Contributor

The problem

There is no way to let a user check the password they just typed.

  • There is no dedicated password element — the variants are Outlined, Filled
    and Bare, and secure is a bool on the shared BaseTextInput.
  • trailing-icon renders as a bare Image (iOS) / MaterialIcon (Android)
    with no gesture attached.
  • BaseTextInput exposes no trailing-press callback.

So the only route open to an app is a separate Show / Hide button beside the
input, bound to a component property.

Why it matters

That workaround is worse than it looks. Every tap crosses the bridge, runs a
PHP handler, and republishes the tree — to flip a boolean that PHP has no
business knowing, next to a field PHP is specifically not told the contents
of. It also puts the control outside the field, where nobody looks for it,
because every platform password field puts it inside.

And the thing it works around is a real accessibility problem: a masked field
with no way to check it is hostile to anyone typing a long generated password
on a phone keyboard.

The approach

revealable, opt-in, alongside secure:

<native:outlined-text-input label="Password" secure revealable native:model="password" />
OutlinedTextInput::make()->secure()->revealable();

The revealed flag is local native state on both platforms@State on
iOS, remember { mutableStateOf } on Android — and that is the whole safety
argument for the feature, not an implementation detail. Because it never
crosses the bridge it cannot republish the tree, cannot perturb text /
lastSentValue, cannot trip the sync_mode state machine, and cannot move
the caret. It is also not the sort of state that should be persisted or
round-tripped anywhere near a password.

iOS re-asserts focus after the swap, deliberately. SecureField has no
unmasked mode, so revealing means building a TextField instead; SwiftUI
treats those as different views, tears the old one down, and first responder
goes with it. Without re-asserting focus on the next runloop turn, the
keyboard drops away every time the eye is tapped. Reading isFocused before
the toggle matters for the same reason.

Selection reporting stays off throughout: selectionEnabled is gated on
!secure, so a revealed password reports no caret offsets either.

The off path is byte-identical. On iOS the toggle is appended as a
ViewModifier that returns its content untouched when disabled, rather than
woven into the existing chain — so a field that doesn't ask for an eye keeps
the exact view tree it had. On Android it is a trailingIcon slot builder
returning null in the same case.

Two contract points that want a maintainer's opinion

1. Honored on outlined + filled only; a documented no-op on bare.
bare-text-input draws no chrome by contract, and on Android its
BasicTextField has no decoration slot to host a control at all. Rather than
let iOS quietly offer something Android cannot, the iOS core takes an explicit
supportsRevealToggle that only the two chrome variants pass. An author using
the chromeless variant is drawing their own decoration and can draw their own
eye.

2. When on, the toggle takes the trailing slot ahead of trailing-icon.
An author who set both asked for the toggle by asking for revealable, and
the icon slot has no other way to express it. A loading spinner still wins
over both — revealToggle is gated on the field being interactive.

Alternatives considered

  • Make it the default rather than opt-in. I would happily argue for this:
    a masked field with no way to check it is a usability and accessibility
    failure, and Safari's own password fields offer the reveal. But it is a
    visible change to every password input already shipped, and it deserves its
    own discussion rather than being smuggled in under a feature PR. Flipping
    the default later is a one-line change to the prop read.
  • A generic @trailingIconTap callback instead. More general, and worse
    here: it routes a password-visibility toggle through PHP, which is the exact
    cost this removes, and it makes every app reimplement the focus-restoration
    dance.
  • Draw the eye in each variant renderer instead of the shared core. The
    focus re-assertion needs @FocusState, which the core owns and does not
    expose. Three renderers would each need their own copy of the state and the
    runloop deferral.
  • Overlay the button on the field rather than putting it in an HStack.
    Avoids any layout change at all, but long values then run underneath the
    eye. The HStack only exists when the feature is on.

What I verified / what I could not

Verified:

  • iOS type-checks against the real SDK. All of resources/ios/*.swift
    compiled together with the core NativeRender sources from a real app
    install:
    swiftc -typecheck -sdk $(xcrun --sdk iphonesimulator --show-sdk-path) -target arm64-apple-ios18.2-simulator.
    Clean; no new warnings.
  • Full Pest suite passes (222 tests), including five new CollectorElementsTest
    cases: the Blade attribute across all three variants, the fluent API, and
    that the prop is omitted when not set (absence is what keeps every
    existing secure field unchanged).
  • pint --test passes; php -l on the changed PHP.
  • Icon names checked against the catalogs this repo ships:
    eye.fill / eye.slash.fill in resources/icons/sf-symbols.json,
    visibility / visibility_off in resources/icons/material-icons.json.

Could not verify — and this is the branch where it matters most:

  • No runtime test of the interaction. The focus-restoration behavior, the
    keyboard not flapping on toggle, and whether the caret survives the
    SecureFieldTextField swap are all things that need a device.
    mobile-ui@main does not currently register most of its components against
    the released core — the manifest's blade paths say
    Native\Mobile\Edge\Components\Text while core ships
    Native\Mobile\Edge\Components\Native\Text, so 25 of 59 entries fail
    class_exists() and are skipped silently, taking <button> and every text
    input with them (52 registered components → 27). I could not build an app
    against this branch to tap the eye in.

    The equivalent iOS behavior is running on a device in an app pinned to
    an older mobile-ui, which is where the shape of this came from — the focus
    re-assertion in particular exists because the keyboard visibly dropped
    without it. That is evidence the design works, not evidence this diff does.

  • Android is entirely compile-unverified, and it is the larger half of the
    risk here: a new TextInputProps field, a slot builder using IconButton +
    MaterialIcon, and a visualTransformation overload. Please have someone
    with a Compose build run it before merging.

  • Layout impact when the toggle is on. The iOS eye uses
    .nuiMinTapTarget() (the repo's 44×44 HIG helper), which will make a md
    field a few points taller than its 41pt content height. That is correct for
    a tappable control and only affects fields that opted in, but it is a real
    visual change I could not eyeball.

There is no way to let a user check the password they just typed. `secure` is
a bool on the shared base, `trailing-icon` renders as a bare `Image` with no
gesture attached, and `BaseTextInput` exposes no trailing-press callback — so
the only route open to an app is a separate Show / Hide button next to the
input, bound to a component property. That costs a bridge round-trip and a
full republish on every tap, to flip a boolean that PHP has no business
knowing, next to a field PHP is specifically not told the contents of.

`revealable` puts the eye inside the field, where the platform puts it.

The revealed flag is local native state on both platforms — `@State` on iOS,
`remember { mutableStateOf }` on Android — and that is the whole safety
argument for the feature, not an implementation detail. Because it never
crosses the bridge it cannot republish the tree, cannot perturb `text` /
`lastSentValue`, cannot trip the `sync_mode` state machine, and cannot move
the caret. It is also not the sort of state that should be persisted or
round-tripped anywhere near a password.

iOS re-asserts focus after the swap, deliberately. `SecureField` has no
unmasked mode, so revealing means building a `TextField` instead; SwiftUI
treats those as different views, tears the old one down, and first responder
goes with it. Without the re-assert on the next runloop turn the keyboard
drops away every time the eye is tapped. Reading `isFocused` BEFORE the
toggle matters for the same reason.

The iOS toggle is appended as a `ViewModifier` that returns its content
untouched when the feature is off, rather than woven into the existing chain,
so a field that doesn't ask for an eye keeps the exact view tree it had. On
Android it is a `trailingIcon` slot builder returning null in the same case.

Two contract points worth flagging for review:

- The toggle is honored on `outlined-text-input` and `filled-text-input`
  only. `bare-text-input` draws no chrome by contract, and on Android its
  `BasicTextField` has no decoration slot to put a control in — so rather
  than let iOS quietly offer something Android cannot, the iOS core takes an
  explicit `supportsRevealToggle` that only the two chrome variants pass.
  The prop is a documented no-op on the chromeless variant.
- When it is on, the toggle takes the trailing slot ahead of the author's
  `trailing-icon`. An author who set both asked for the toggle by asking for
  `revealable`, and the icon slot has no other way to express it. A loading
  spinner still wins over both: `revealToggle` is gated on the field being
  interactive.

Opt-in, so nothing changes for any existing secure field. I would happily
argue for making it the default — a masked field with no way to check it is a
usability and accessibility failure, and Safari's own password fields offer
the reveal — but that is a visible change to every password input already
shipped, and it belongs in its own discussion rather than smuggled in here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant