Add an in-field reveal toggle for secure inputs - #61
Open
CodyPChristian wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
There is no way to let a user check the password they just typed.
and Bare, and
secureis a bool on the sharedBaseTextInput.trailing-iconrenders as a bareImage(iOS) /MaterialIcon(Android)with no gesture attached.
BaseTextInputexposes 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, alongsidesecure:The revealed flag is local native state on both platforms —
@StateoniOS,
remember { mutableStateOf }on Android — and that is the whole safetyargument for the feature, not an implementation detail. Because it never
crosses the bridge it cannot republish the tree, cannot perturb
text/lastSentValue, cannot trip thesync_modestate machine, and cannot movethe 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.
SecureFieldhas nounmasked mode, so revealing means building a
TextFieldinstead; SwiftUItreats 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
isFocusedbeforethe toggle matters for the same reason.
Selection reporting stays off throughout:
selectionEnabledis 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
ViewModifierthat returns its content untouched when disabled, rather thanwoven 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
trailingIconslot builderreturning
nullin 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-inputdraws no chrome by contract, and on Android itsBasicTextFieldhas no decoration slot to host a control at all. Rather thanlet iOS quietly offer something Android cannot, the iOS core takes an explicit
supportsRevealTogglethat only the two chrome variants pass. An author usingthe 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, andthe icon slot has no other way to express it. A
loadingspinner still winsover both —
revealToggleis gated on the field being interactive.Alternatives considered
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.
@trailingIconTapcallback instead. More general, and worsehere: 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.
focus re-assertion needs
@FocusState, which the core owns and does notexpose. Three renderers would each need their own copy of the state and the
runloop deferral.
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:
resources/ios/*.swiftcompiled together with the core
NativeRendersources from a real appinstall:
swiftc -typecheck -sdk $(xcrun --sdk iphonesimulator --show-sdk-path) -target arm64-apple-ios18.2-simulator.Clean; no new warnings.
CollectorElementsTestcases: 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 --testpasses;php -lon the changed PHP.eye.fill/eye.slash.fillinresources/icons/sf-symbols.json,visibility/visibility_offinresources/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
SecureField→TextFieldswap are all things that need a device.mobile-ui@maindoes not currently register most of its components againstthe released core — the manifest's
bladepaths sayNative\Mobile\Edge\Components\Textwhile core shipsNative\Mobile\Edge\Components\Native\Text, so 25 of 59 entries failclass_exists()and are skipped silently, taking<button>and every textinput 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
TextInputPropsfield, a slot builder usingIconButton+MaterialIcon, and avisualTransformationoverload. Please have someonewith 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 amdfield 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.