Repository navigation
fix(mobile): restore notification settings without T3 Connect - #54
Merged
NicholasZolton merged 1 commit intoNov 4, 2026
Merged
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
Platform.OSto… · Newvi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true)is not restored, andharness.cloudConfiguredis… · NewconnectionRowsreturns both “Environments” and “Notifications”, which are settings entries but… · New
What changed in this PR
Restores access to mobile notification settings in builds without T3 Connect by reusing the same “Connections” rows across both the cloud-configured and local-only settings lists, plus adds regression coverage and documentation.
Changes:
- Unifies “Environments” + “Notifications” rows via a shared helper used by both settings list variants.
- Adds a regression test ensuring “Notifications” remains visible with and without cloud config, and wires it into the mobile test list.
- Updates wiki docs/log to record the regression and its fix.
| File | Description |
|---|---|
| wiki/paired-notifications.md | Documents the local-only Settings entry regression + sources/links for traceability. |
| wiki/log.md | Adds an entry describing the compatibility boundary and regression coverage. |
| apps/mobile/vite.config.ts | Includes the new SettingsRouteScreen test in the React test allowlist. |
| apps/mobile/src/features/settings/SettingsRouteScreen.tsx | Reuses shared connection rows so Notifications appears in both local-only and configured lists. |
| apps/mobile/src/features/settings/SettingsRouteScreen.test.tsx | Adds a regression test for Notifications visibility independent of cloud config. |
Review effort: Lite
| }), | ||
| ); | ||
|
|
||
| vi.mock("react-native", () => ({ View: Children, Platform: { OS: "ios" } })); |
Comment on lines
+94
to
+102
| it.each([false, true])("shows notification settings with cloudConfigured=%s", (cloudConfigured) => { | ||
| vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); | ||
| harness.cloudConfigured = cloudConfigured; | ||
| using screen = renderSettings(); | ||
| const notifications: HTMLButtonElement[] = Array.from( | ||
| screen.container.querySelectorAll("button"), | ||
| ).filter((button) => button.textContent === "Notifications"); | ||
| expect(notifications).toHaveLength(1); | ||
| }); |
Comment on lines
+141
to
158
| function connectionRows(count: number): SettingsListRow[] { | ||
| return [ | ||
| { | ||
| id: "SettingsEnvironments", | ||
| icon: "desktopcomputer", | ||
| label: "Environments", | ||
| value: `${count}`, | ||
| valuePosition: "trailing", | ||
| target: "SettingsEnvironments", | ||
| }, | ||
| { | ||
| id: "SettingsNotifications", | ||
| icon: "bell.badge", | ||
| label: "Notifications", | ||
| target: "SettingsNotifications", | ||
| }, | ||
| ]; | ||
| } |
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.
Paired Android users could no longer find notification settings after the November upstream sync. The fork added a Notifications entry for builds without T3 Connect on October 10, but the incoming settings-list refactor omitted it from the local-only list. This made the existing paired-notification opt-in unreachable.
Use the same Environments and Notifications rows in both settings lists. The existing notification screen still handles paired delivery without cloud configuration, and iOS still requires T3 Connect for push. No delivery, credential, server, or wire-contract changes are included.
Test plan
check:changed: exceeded the local execution timeout after format, lint, and both Knip passes; focused mobile checks passed separately.Rollout
The private Android APK needs rebuilding and reinstalling to expose the menu. Its existing Preview package, Firebase app configuration, private signing identity, and disabled OTA update source must be retained. Updating the server alone does not update this menu.
calm-mushroom-duck