Foundations, accessibility, and objective fixes across the app - #137
Merged
Conversation
Falling back to the defaults means the next settings change writes over whatever is still on disk, so a single transient read failure — a locked database, for instance — would cost the user their real settings. Give the read a second chance before giving up on it. Damaged text is not retried: it will not parse on a second read either. The write paths swallowed their errors completely. Leave a trace instead; the app is offline, so a log is the only channel available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`expo/tsconfig.base` sets no `strict`, no `strictNullChecks` and no `noImplicitAny`, so none of the guarding the code already does was being checked. The codebase was written in a strict style throughout — it compiles clean with the flag on, with no source changes needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nineteen sites indexed an array without checking the result. One was a real latent crash: `exercise-screen.tsx` read `currentStep.id` unguarded, while `useExerciseLoop` types it as possibly undefined. Only the one-second floor on the inhale and exhale durations kept it from happening, and that protection was accidental. Most of the rest are better expressed as types than as guards: - `PatternSteps` is now a four-element tuple, shared by `PatternPreset` and `CustomPatternSteps`. The four-step shape was already an invariant everywhere; now it is stated once. - `patternPresets` is a non-empty tuple, so the code that falls back to its first entry no longer needs an assertion. - `normalizePersistedSettingsState` walks the duration limits instead of the default steps, which removes a parallel-array index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`easing` was assigned after the config spread in `animate`, so every easing a caller passed was silently discarded. The breathing circle, the step label, the timer, the dots, the interlude and the completion screen all ran the same `Easing.inOut(Easing.quad)` curve whether or not they asked for it. Move it above the spread and keep it as the documented default. `useNativeDriver` stays below on purpose: everything animated here is opacity or transform, and that is worth enforcing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`colors.ts` was a palette named by hue and step, with no light/dark pairing, so nine files re-derived the mapping by hand with `isDarkMode && styles.xDark`. That is the direct cause of every missing dark variant in the app: the tagline, the "or" separator, the paused description and the interlude subtitle all kept their light colour on the dark background, while the titles beside them did not. Add `themeColors`, which says where a colour belongs rather than what it is, with a value per scheme for background, surface, text, textSecondary, border and control. `useThemeColors()` returns the set for the active scheme. A role that exists here cannot be left without a dark value by accident. The new secondary and control values were chosen to clear the WCAG thresholds that the old ones missed, and `colors.test.ts` computes the ratios and asserts them, so a later colour change cannot quietly drop below the line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…h schemes Five elements carried a light-mode colour with no dark counterpart, so they kept it on the dark background: the home tagline, the "or" separator, the paused description and the interlude subtitle all measured 3.75:1 there and 4.36:1 in light, against a 4.5:1 requirement. The exercise screen's close button was worse. Its icon was the hardcoded CSS keyword `lightgray` and its ring was `gray-300`, both unconditional — 1.37:1 and 1.35:1 on the light background, against the 3:1 a control outline needs. It is the only way out of a running session, and in light mode it was very nearly invisible. All six now read their colour from `useThemeColors()`, so each has a value per scheme and the ratios are the ones asserted in `colors.test.ts`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add two presets that the list was missing: - `coherent` (5.5s in, 5.5s out) — coherent/resonant breathing at about 5.5 breaths per minute. - `extended-exhale` (4s in, 6s out) — a longer exhale than inhale, the simplest way to settle. Also rename the `deep-calm` preset to "4-7-8 Deep Calm". It is the 4-7-8 technique, but the old name did not say so, and "4-7-8" is the term a user searches for. The preset `id` values do not change. `normalizePersistedSettingsState` in `src/stores/settings-state.ts` validates the stored id against this list, so a changed id would silently reset that user to the Square pattern. Only the `name` changed. The name is "4-7-8 Deep Calm" and not "4-7-8 (Deep Calm)" because the settings screen appends the step durations in parentheses. A parenthesised name gives "4-7-8 (Deep Calm) (4-7-8-0)", which reads badly and also stops the `.maestro/flows/settings-persistence.yaml` assertion from matching. "4-7-8 Deep Calm (4-7-8-0)" reads correctly and keeps that flow green. The array is now sorted by display name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bell mode used `cueBell1` for both `breatheIn` and `breatheOut`, so a user with the eyes closed — the reason to choose the bell mode — could not hear which way to breathe. `cueBell2` was on `hold`, and the `awake` and `ujjayi` presets have no hold step, so in those presets the second bell never played at all. The inhale keeps `cueBell1` and the exhale now uses `cueBell2`. `hold` reuses the inhale bell, because a third distinct sound is not available and the hold is the less important cue. Add a test that asserts the two directions do not share a source. It fails against the old mapping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remove 1.83 MiB across 19 files that nothing references: - 15 of the 20 font faces. `src/assets/fonts.ts` registers only GeneralSans Regular/Medium and Lora Medium/SemiBold, and `src/design/typography.ts` is the only consumer of a family name. - `paulbreatheout.mp3` and `paulthein.mp3`. `src/assets/sounds.ts` maps the other 9 files in `assets/audio/`; these two are earlier takes. - `assets/images/background-stars-horizontal-old.png`. `images.ts` maps only the non-`-old` file. - One of the two app icons. `assets/ios-icon.png` and `assets/android-icon.png` were byte-identical (verified with `cmp` and `md5`). They are now a single `assets/icon.png`, which the root `icon`, `ios.icon` and `android.icon` keys all point at. The neutral name replaces two per-platform names for one shared file. `GeneralSans-Light.otf` is deliberately kept although it is not registered. `src/screens/home-screen/home-screen.tsx` sets `fontWeight: "300"` on `fontFamilies.regular`, and 300 is exactly the Light face. Registering that face is the better fix than deleting it, but that change belongs to `src/design` and `src/screens`. This is repository weight, not binary size. `expo-updates` is not installed, so `assetBundlePatterns` is inert, and Metro bundles only what a `require()` reaches — these files were never in a shipped build. Do not read this as an app-size win. Verified: every `require()` in `src/assets` and every `./assets/` path in `app.json` still resolves; `git grep` over all tracked files finds no remaining mention of any deleted file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JUDGEMENT CALL — REVIEW BY EAR. Revert this commit alone if the result is not good enough. The files were encoded inconsistently. `paulinhale.mp3` was stereo 192 kbps while `paulexhale.mp3` and `paulhold.mp3` were mono 128, so the timbre shifted inside a single breath. Laura's three were stereo 320. The six voice cues are now mono 128 kbps and the three bells are stereo 160 kbps. 546 KB -> 332 KB, a 39.1% saving. These files are `require()`d by `src/assets/sounds.ts`, so unlike the asset deletion this does reduce the shipped binary. Every duration was measured with `afinfo` before and after. Eight files are unchanged to the microsecond. `paulinhale.mp3` moved 26.1 ms, which is one MPEG-1 Layer III frame at 44.1 kHz and comes from the stereo to mono conversion. That is inside the 50 ms budget, so no file was reverted. All nine decode without an ffmpeg error. The tradeoff the owner has to judge: the voice lines were bought from a voice actor, and taking Laura from 320 kbps to 128 is a decision about her recordings, not a measurement. Mono is defensible because a breathing cue carries no stereo image, and 128 kbps mono is a normal rate for speech, but only listening can confirm it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`settings-ui.d.ts` declared a `SettingsUI` shape that nothing was ever assigned
to, so TypeScript never compared the three platform files with each other. Three
behaviours had already drifted apart, and nothing noticed.
Move the contract into `settings-ui.types.ts` as `SettingsUIModule` and annotate
the exported object in all three implementations with it, so the compiler checks
them. Add a parity test that renders the iOS and web implementations through the
same assertions: every component present, the field name visible on a picker,
both stepper buttons named for a screen reader, the stepper test ids exposed, and
a disabled radio row reporting its state.
The test caught the web picker bug it was written to catch: `PickerItem` spread
the shared props into each option row and then overrode `label` with the option's
own, so the field name ("Voice") never reached the screen and the user saw a set
of unnamed choices. It now renders once above the options.
Also renames `hideBottomBorderAndroid` to `hideBottomBorder` — it was read by the
web implementation and ignored by Android, which is the opposite of its name.
Android is not rendered in the test; its components need the native runtime, so
the compiler carries its half of the contract. Its missing stepper accessibility
names are documented in place: they cannot be fixed with the current @expo/ui API.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-improvements # Conflicts: # src/assets/pattern-presets.ts
iOS reports "inactive" for the Control Center, the Notification Center, the app switcher and an incoming call banner. The app stays on the screen in all of them, thus the user came back to a live session and found it stopped behind a Resume button. The exercise now stops only when the app really goes to the background. The timer counts the inactive time as well, because the breathing loop, the voice and the haptics keep running there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The defaults are `shouldFollowSystemDarkMode: true` with `theme: "light"`. A user on a dark phone saw a dark app, turned the switch off intending to keep dark, and `resolveColorScheme` returned the stored "light" — so the app flipped to light in front of them, with a layout animation, and they had to find the theme picker and set dark again by hand. Write the currently resolved scheme into `theme` before the switch goes off, so turning it off changes nothing until the user asks for a change. Also renames the two custom-pattern "Hold" rows to "Hold after inhale" and "Hold after exhale". They were identical, so a screen reader announced "Hold, Time in seconds, Decrease Hold, Increase Hold" twice with nothing to tell them apart. No Maestro flow refers to either label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The clock reaches 00:00 before the exercise stops, because the exercise continues to the end of the current breath. The timer faded away there and left the accessibility tree, thus no user knew how long the exercise still ran. With a custom pattern this silence lasts minutes. The timer now crossfades to "Last breath" and stays visible and readable by a screen reader until the exercise stops. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The label animation lasted 400 ms at every step length. On a step of 800 ms or less the fade out started before the fade in ended, and because both fades share one animated value the fade out stopped the fade in. `Animated.parallel` stops all of its animations together, thus it stopped the breathing circle as well: at a step of 500 ms the circle stopped at 98 ms and held 0.08 instead of 1.00, the label reached an opacity of 0.12, and the composite still reported that it finished. The label animation now scales with the step and the breathing circle no longer stops with it. A hold of half a second is available through the custom pattern settings, thus this defect is reachable today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A gap between two ticks that is longer than one second comes from a stall of the JavaScript thread. The timer discarded the full gap, thus the session lost that time and ran past the time limit that the user selected. On a slow device every stall made the error larger. The timer now counts the limited part of the gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`useKeepAwake` was inside the running fragment, thus the countdown of 4.4 seconds, the paused screen and the completion screen had no protection. A screen that locks during the countdown pauses the session before the first breath. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The step label, the countdown and the completion are animated texts. A screen reader said nothing when they changed, thus a user without sight could not follow the session: only a swipe to the same element every few seconds gave the current step. The step label and the countdown are now polite live regions for Android and announce themselves on iOS, which has no live regions. The label also gives the duration of the step, e.g. "Inhale, 4 seconds". The last breath and the end of the session announce themselves on both platforms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The exercise rotated and translated eight circles, sixteen in dark mode, across the width of the screen for the whole session, and the app had no check and no switch for it. That is the motion profile that starts vestibular symptoms. With reduce motion on, one circle in the centre of the screen changes only its opacity and its size, and the step label, the dots and the countdown only fade. The timing of the exercise does not change: the voice and the haptic carry the rhythm on their own, thus the user loses nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The visuals carry the whole exercise, thus a user of a screen reader who disabled the guided voice had no channel that works without sight. The exercise now uses the default voice when a screen reader runs and the voice is disabled. The stored setting does not change: this is an override of the render and the user keeps the choice made in the settings screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The index of an array gives a value that can be missing. The exercise loop always gives a step, but the guards make the two places safe if the project makes the compiler stricter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The system answers the questions about the accessibility settings asynchronously, and the components of the exercise mount at different times: the breathing animation mounts 4.4 seconds after the countdown. A component that mounted later therefore drew one frame with the motion that the user did not want. The answers now stay in the module, thus every component that mounts later starts with the answer that the screen already has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-improvements # Conflicts: # src/screens/exercise-screen/exercise-screen.tsx # src/screens/exercise-screen/interlude.tsx # src/utils/loop-animations.ts
The sky drifts across the screen for as long as it is mounted, on both the home and the exercise screens. After the rest of the app learned to honour Reduce Motion, it was the only continuous motion left, which defeats the point of the setting for the users who need it most — the exercise screen is where they spend the whole session. The star field itself is the point, not the drift, so it stays and only the animation stops. Applied to the native and the web implementations. `use-accessibility-preferences` moves from `screens/exercise-screen/` to `utils/`. It is now read by both screens, and a home-screen file reaching into the exercise screen for it would be the wrong direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`expo-splash-screen` treats `image` as a logo to centre, not a full-bleed background. `imageWidth` was unset, so the generated storyboard drew `splash.png` inside a 100x100 pt box, and `resizeMode: "cover"` applied within that box rather than to the screen. The launch therefore showed the artwork twice: a tiny centred crop from the native storyboard, then the same image full-bleed from `SplashScreenManager`. The JS overlay has to stay — it holds the splash until the home screen is ready, which in dark mode waits for the star field to decode, and the native splash cannot know about that. So the native side keeps the background colour only and hands over without drawing anything of its own. Android is unaffected: it keeps its own `drawable.icon`, and the system splash on Android 12 and later always shows an icon. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The native launch screen and the JS overlay rendered the same artwork differently, so the hand-off between them was visible. `enableFullScreenImage_legacy` restores the full-bleed rendering on iOS, so the storyboard now draws `splash.png` at `scaleAspectFill` across the whole screen — exactly what `SplashScreenManager` draws a moment later. The two layers are the same image at the same size, so there is nothing to see when one replaces the other. Both layers also gain a dark variant. On the native side that is a config key: the plugin generates a two-appearance colorset and imageset, and the system picks. `SplashScreenManager` follows the same signal — the *system* appearance rather than the app's resolved theme, because the native layer can only follow the system and the settings store has not hydrated yet at that point. `assets/splash-dark.png` is derived from `splash.png`: the same wordmark in the same position, recoloured to near-white on the dark background. Note `enableFullScreenImage_legacy` is documented as a transition helper that will eventually be removed. When it goes, the durable replacement is a centred wordmark logo used by both layers, which would also fix the iPad crop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Follow-up to #136. 28 commits, grouped below. Two of them are deliberately easy to drop — see Judgement calls at the end.
bun run validate:staticpasses at every commit (80 tests, up from 38). All four Maestro flows pass against an Android release build with R8 enabled — the shrunk build nothing had exercised since the Compose settings screen landed. The iOS splash work was verified on a clean simulator in both appearances.Foundations
These went first because they change the ground everything else stands on.
strict: true.expo/tsconfig.basesets nostrict, nostrictNullChecks, nonoImplicitAny, so none of the guarding the code already did was checked. It compiles clean with the flag on — no source changes were needed.noUncheckedIndexedAccess. 19 sites. One was a real latent crash:exercise-screen.tsxreadcurrentStep.idunguarded whileuseExerciseLooptypes it as possibly undefined; only the one-second floor on inhale/exhale durations prevented it, which was accidental. Most of the rest were better expressed as types —PatternStepsis now a 4-tuple andpatternPresetsa non-empty tuple, so the fallback needs no assertion.animate()honours itseasingargument. It was assigned after the config spread, so every easing a caller passed was silently discarded and the whole app ran one curve.colors.tswas a palette with no light/dark pairing, so nine files re-derived the mapping by hand withisDarkMode && styles.xDark— the direct cause of every missing dark variant.themeColorsgives each role a value per scheme, andcolors.test.tscomputes WCAG ratios and asserts them.settings-ui.d.tsdeclared a shape nothing was assigned to, so TypeScript never compared the three platform files. Three behaviours had already drifted. All three now satisfySettingsUIModule, and a parity test renders iOS and web through the same assertions.Bugs fixed
label, so "Voice" never reached the screen. Caught by the new parity test on its first run.shouldFollowSystemDarkMode: truewiththeme: "light", so a user on a dark phone who switched it off to keep dark was flipped to light.lightgrayon the light background is 1.37:1 against a 3:1 requirement, and it is the only way out of a running session.Animated.staggerstarted the label fade-out atduration − 400 mswith both fades sharing one value; below 800 ms the fade-out interrupted the fade-in andstopTogethertook the circle with it. Measured at a 500 ms step: the circle held 0.078 instead of 1.00 and stopped at 98 ms — while the composite still reportedfinished: true. 800 ms was broken too.AppStatelistener fired on any non-activestate; iOS reportsinactivefor Control Center, the app switcher and unanswered call banners.cueBell2never played at all in the presets with no holds.expo-splash-screentreatsimageas a logo to centre; withimageWidthunset it renderedsplash.pngin a 100 pt box, then the JS overlay drew the same artwork full-bleed. Both layers now draw the same thing, and both gained a dark variant.Accessibility
The app had zero uses of
accessibilityLiveRegion,announceForAccessibilityandAccessibilityInfo, so a session could not be followed without sight, and eight rotating circles (sixteen in dark mode) ran regardless of Reduce Motion.react-native-webanswerstruetoisScreenReaderEnabled()for every browser.Last breathrather than fading out of the accessibility tree.Content and assets
deep-calmto 4-7-8 Deep Calm so the best-known pattern in the app is findable. Preset ids are unchanged —normalizePersistedSettingsStatevalidates the stored id against this list, and a changed id would silently reset every existing user to Square.expo-updatesis not installed, soassetBundlePatternsis inert and Metro bundles only whatrequire()reaches.Judgement calls for the reviewer
eb8294bre-encodes the audio. Laura's lines go from stereo 320 to mono 128 kbps. Defensible for speech, but these are paid voice-actor recordings and it is a listening call, not a measurement. Kept as its own commit sogit revert eb8294bdrops it alone.enableFullScreenImage_legacyis documented by the plugin as a transition helper that will be removed. It is one line and it works today. The durable replacement is a centred wordmark logo used by both layers, which would also fix the iPad crop — that changes how the launch looks, so it is left as a decision.Known gap, not fixed
Android's stepper buttons still have no accessible name, so TalkBack reads "−" and "+" with no context. It cannot be fixed with the current
@expo/uiAPI: thesemanticsmodifier takes onlycontentType,FilledTonalButtonexposes no content description, andIcon— the one component that accepts one — needs a vector drawable rather than a text glyph. The reason is written into the file at the call site.Not attempted
Maestro in CI, the home screen restyle, and the breathing curves. The last two are taste work and are better done with eyes on them.