fix: give the two single-field screens their initial focus, and check 200% text on a device
Phase 3, final step, of docs/material-design-conformance.md. The tree had **zero** uses of
`FocusRequester`, `LocalFocusManager` or `focusProperties`, so no screen defined where
keyboard focus starts.
**Two places get it, and only two.** M3's flow guidance asks for an initial focus per
screen and, for a dialog, that "focus is set to the dialog component, likely to a specific
interactive element within the dialog such as a text input field":
- `StartDirectMessageToNpubOrNip05Dialog` -- one field and two buttons. Without this the
dialog opens with nothing focused, so a keyboard or switch user tabs in from wherever
focus happened to be.
- The desktop `PassphraseGate` -- the first screen of the desktop app, whose entire
content is one field, and where there is no tap to give it focus. Somebody who opens
the app and starts typing should not have to reach for the mouse first.
The other seven text-field screens deliberately do **not** auto-focus. Requesting focus
raises the software keyboard, and on a screen that leads with content somebody wants to
read -- AddArtifact's chapter list, WriteNewNote's reply preview -- that covers the thing
they came for. M3 asks for the initial focus to be *defined*, not for a field to be
grabbed; on those screens the definition is "the top of the content".
**Large text verified on a device rather than reasoned about.** Two passes:
A static one first, since the failure mode is a fixed height around text. All 23 fixed
vertical dimensions outside `Spacer`s are icons, images and progress indicators -- 12 to
40dp `.size()` calls, a 200dp image, a 180dp `heightIn` cap. Nothing wraps text in a fixed
box.
Then at `font_scale 2.0` on an API 36 emulator, three screens: onboarding, the message
list, and a chat room. All reflow without clipping. The chat room is the useful one --
system messages wrap to two lines and their timestamps and chevrons stay aligned, the
composer keeps its full width, and the transcript stays readable. `font_scale` was put
back to 1.0 afterwards.
The physical device attached to this machine was left alone. `font_scale` is a
system-wide setting and changing it on somebody's actual phone to test an app is not a
reasonable thing to do; a fresh emulator was booted for it instead.
**An unrelated flaky test, measured and left alone.** `ChronicleApplyJvmTest > an answered
catch-up leaves one line, whatever it took to deliver` failed once during this commit's
verification with
expected:<[chronicleRequested, chronicleReceived]> but was:<[chronicleReceived, chronicleRequested]>
and reproduces at **1 failure in 8** consecutive `--rerun` invocations on this tree. Both
transcript lines are written within the same second and the DAO's ordering has no
documented tie-break, so either order can come back. That is chronicle and database code;
nothing in this branch touches it. Whether it is a test bug or a real one -- two lines
swapping places in a user's transcript on reload would be a defect -- wants deciding by
somebody in that code, so it is filed rather than patched here.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. Focus and
window insets are properties of a running composition; there is no Compose UI test
infrastructure here, and a test asserting the modifier is present would restate the diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -43,6 +43,9 @@ import fr.acinq.phoenix.utils.Parser
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.launch
|
||||
import press.mantra.compose.ui.theme.spacing
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.ui.focus.FocusRequester
|
||||
import androidx.compose.ui.focus.focusRequester
|
||||
|
||||
@Composable
|
||||
fun StartDirectMessageToNpubOrNip05Dialog(
|
||||
@@ -67,6 +70,15 @@ fun StartDirectMessageToNpubOrNip05Dialog(
|
||||
shape = RoundedCornerShape(16.dp),
|
||||
) {
|
||||
val textFieldState = rememberTextFieldState()
|
||||
|
||||
// M3's flow guidance for a dialog: "Focus is set to the dialog component,
|
||||
// likely to a specific interactive element within the dialog such as a text
|
||||
// input field." This dialog is one field and two buttons, so the field is
|
||||
// unambiguous -- and without this the whole dialog opens with nothing
|
||||
// focused, which for a keyboard or switch user means tabbing in from
|
||||
// wherever focus happened to be.
|
||||
val npubFieldFocus = remember { FocusRequester() }
|
||||
LaunchedEffect(Unit) { npubFieldFocus.requestFocus() }
|
||||
Column(
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
horizontalAlignment = Alignment.CenterHorizontally,
|
||||
@@ -84,6 +96,7 @@ fun StartDirectMessageToNpubOrNip05Dialog(
|
||||
textAlign = TextAlign.Center
|
||||
)
|
||||
TextField(
|
||||
modifier = Modifier.focusRequester(npubFieldFocus),
|
||||
state = textFieldState,
|
||||
placeholder = {
|
||||
Text("Input npub... or nip05")
|
||||
|
||||
@@ -28,6 +28,9 @@ import androidx.compose.ui.window.Window
|
||||
import androidx.compose.ui.window.application
|
||||
import androidx.compose.ui.window.rememberWindowState
|
||||
import androidx.navigation.compose.rememberNavController
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.ui.focus.FocusRequester
|
||||
import androidx.compose.ui.focus.focusRequester
|
||||
import fr.acinq.phoenix.PhoenixGlobal
|
||||
import fr.acinq.phoenix.security.JvmKeyStore
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
@@ -108,6 +111,13 @@ private fun PassphraseGate(appDir: File, onUnlocked: () -> Unit) {
|
||||
var error by remember { mutableStateOf<String?>(null) }
|
||||
val scope = rememberCoroutineScope()
|
||||
|
||||
// The first screen of a desktop app, whose entire content is one field. M3 asks for
|
||||
// an initial focus to be defined per screen; here there is only one candidate, and on
|
||||
// a desktop there is no tap to give it focus -- somebody who opens the app and starts
|
||||
// typing their passphrase should not have to reach for the mouse first.
|
||||
val passphraseFocus = remember { FocusRequester() }
|
||||
LaunchedEffect(Unit) { passphraseFocus.requestFocus() }
|
||||
|
||||
fun submit() {
|
||||
if (busy || passphrase.isEmpty()) return
|
||||
busy = true
|
||||
@@ -156,7 +166,7 @@ private fun PassphraseGate(appDir: File, onUnlocked: () -> Unit) {
|
||||
visualTransformation = PasswordVisualTransformation(),
|
||||
keyboardOptions = KeyboardOptions(imeAction = ImeAction.Go),
|
||||
keyboardActions = KeyboardActions(onGo = { submit() }),
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().focusRequester(passphraseFocus),
|
||||
)
|
||||
error?.let {
|
||||
Text(
|
||||
|
||||
@@ -550,66 +550,59 @@ visible.
|
||||
|
||||
### Phase 3 — the accessibility floor
|
||||
|
||||
**Why here.** These are defects, not polish, and one of them hides the app's
|
||||
most important rows. It goes before the adaptive work so the fixes are made once
|
||||
rather than per breakpoint.
|
||||
**Why here.** These are defects, not polish, and one of them hides the app's most
|
||||
important rows. It goes before the adaptive work so the fixes are made once rather than
|
||||
per breakpoint.
|
||||
|
||||
**Work.**
|
||||
**Built.** Four commits.
|
||||
|
||||
1. **Fix the seven measured pairings.** Take them in the order of the table.
|
||||
`ProposalListScreen.kt:228` needs the `ListItem` colours derived from the
|
||||
card's container, not left at `onSurface`:
|
||||
1. **Nine contrast failures**, all of them colour reached for at the call site rather
|
||||
than derived from what it sits on. The worst was `ProposalListScreen`: a `ListItem`
|
||||
inside a `Card` with only the container overridden, so the headline took
|
||||
`ListTokens.ItemLabelTextColor` (`onSurface`) against `primaryContainer` — both
|
||||
`#1B1B1B` in the light scheme, **1.00:1**, on exactly the proposals awaiting your
|
||||
signature. Four of five colours on that card were under the floor.
|
||||
|
||||
```kotlin
|
||||
ListItemDefaults.colors(
|
||||
containerColor = Color.Transparent,
|
||||
headlineColor = contentColorFor(cardContainer),
|
||||
supportingColor = contentColorFor(cardContainer),
|
||||
leadingIconColor = contentColorFor(cardContainer),
|
||||
)
|
||||
```
|
||||
Also `HomeScreen`'s top bar at 1.22:1 (override removed), five
|
||||
`onSurfaceVariant.copy(alpha = …)` sites, the LIVE badge's hand-mixed red, and the
|
||||
avatar picker's selection tint. Three colours stay hardcoded behind a
|
||||
`// m3-color-exempt:` marker with a reason at the site: a QR code's modules, a
|
||||
control over an arbitrary photograph, a spinner over a blurhash.
|
||||
|
||||
`HomeScreen.kt:113` should drop its `topAppBarColors` override entirely —
|
||||
the default is `surface`/`onSurface` and is correct. The four
|
||||
`onSurfaceVariant.copy(alpha = 0.5f)` sites should use `onSurfaceVariant` at
|
||||
full opacity, which is already the role for secondary text.
|
||||
2. **Every `.clickable` chain got `minimumInteractiveComponentSize()`** — a no-op above
|
||||
48dp, so applying it everywhere makes the rule checkable rather than measurable. Three
|
||||
of the nineteen were text-sized. `Clickable.kt` got it built in and left
|
||||
`com.machankura`.
|
||||
|
||||
2. **Extend the contrast test to call sites.** Every non-default
|
||||
`containerColor`/`contentColor` pairing in the tree gets a row in a table the
|
||||
test walks. This is what stops the class of bug rather than the instance —
|
||||
the `ListItem`-inside-`Card` case is invisible to a reviewer and obvious to
|
||||
an assertion.
|
||||
3. **Eighteen `contentDescription = null` became a decision.** Fifteen say `Decorative`
|
||||
— same null, but recording that somebody looked. Three carried state the text did not
|
||||
repeat and got real descriptions, including `ProposalListScreen`'s stage icon, which
|
||||
the previous commit had made the *only* cue for a failed proposal on the highlighted
|
||||
card.
|
||||
|
||||
3. **Guarantee the 48dp minimum.** The 33 bare `.clickable` sites either become
|
||||
a real component (`ArticleCard.kt:143`'s author name is a `TextButton`) or
|
||||
gain `Modifier.minimumInteractiveComponentSize()`. `Clickable.kt` gets it
|
||||
built in, moves into `press.mantra`, and its `RectangleShape` default is
|
||||
reconsidered so state layers read.
|
||||
4. **Eight text-field screens lift above the keyboard.** `Scaffold`'s
|
||||
`contentWindowInsets` is `systemBars` and excludes the ime, so a Scaffold alone does
|
||||
nothing about it. `ChatRoomMessagingScreen` is deliberately excluded — its composer
|
||||
already reserves the bottom inset by hand, and combining the two needs a device.
|
||||
|
||||
4. **Triage the 18 null content descriptions.** Each is either genuinely
|
||||
decorative — and then says so with `Modifier.clearAndSetSemantics {}` or a
|
||||
comment — or gets a label. The spec's rule for the labels themselves: name
|
||||
the purpose, not the picture, and never include the role ("Search", not
|
||||
"magnifying glass", never "Search button").
|
||||
5. **Initial focus** on the npub dialog's field and the desktop passphrase field — M3's
|
||||
flow rule that a dialog takes focus on open, and that every screen defines one. Those
|
||||
are the two places where a single field *is* the screen; auto-focusing elsewhere would
|
||||
pop the keyboard over content somebody wants to read first.
|
||||
|
||||
5. **Survive a large font scale.** Test every screen at 200% text size. The
|
||||
pattern to look for is a fixed `height` on a container of text; the 49
|
||||
`height(50.dp)` spacers are safe, but `Modifier.height(…)` around a `Text`
|
||||
is not.
|
||||
**Font scale verified, not assumed.** A static pass found no fixed-height container
|
||||
holding text — all 23 fixed vertical dimensions are icons, images and progress
|
||||
indicators. Then at 200% text size on an API 36 emulator: onboarding, the message list
|
||||
and a chat room all reflow without clipping, system messages wrapping to two lines with
|
||||
their timestamps and chevrons still aligned.
|
||||
|
||||
6. **Keyboard flow for the desktop target.** Initial focus per screen, focus
|
||||
into and back out of the four `ModalBottomSheet`s and the `AlertDialog`, and
|
||||
`Tab` order verified on the nine screens with text fields. The foundations
|
||||
page is explicit that when a dialog opens, focus moves into it, and when it
|
||||
closes, focus returns to what opened it.
|
||||
|
||||
7. `imePadding()` on all nine text-field screens and the npub dialog, not one.
|
||||
|
||||
**Done when** the extended contrast test passes over both scheme families; the
|
||||
audit reports zero unguarded `.clickable`; every `Icon` either has a description
|
||||
or a recorded reason; and every screen is legible at 200% text scale.
|
||||
|
||||
**Risk:** low. Each fix is local and independently verifiable.
|
||||
**Left undone, and why.** Full keyboard traversal on desktop — tab order across the four
|
||||
`ModalBottomSheet`s and the `AlertDialog`, and focus returning to what opened them — is
|
||||
not verified. Compose restores focus on dismissal by default, so the gap is evidence
|
||||
rather than known breakage, and checking it means driving the desktop build by keyboard.
|
||||
The avatar picker's selected state is `secondaryContainer` at 1.65:1 against the surface,
|
||||
which M3 accepts only because its own selected states carry a second cue; this grid has
|
||||
neither an outline nor a checkmark, and adding one is component work.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user