Repository navigation
fix(mobile): refine iOS navigation and conversation details - #8075
Conversation
Signed-off-by: kenny lopez <klopez4212@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM
Findings[MEDIUM]
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf1a89d5b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'offline' => context.colors.outline, | ||
| _ => null, | ||
| }, | ||
| onNativeTitlePressed: openChannelDetails, |
There was a problem hiding this comment.
When an iOS user taps any DM title, this newly opens ChannelDetailsPage, whose lifecycle card offers “Leave channel” for every isMember conversation and submits kind 9022. The relay handles that event by setting the caller’s membership as removed, but DMs are modeled with immutable participant sets and their intended dismissal path is hide_dm; reopening the same participant set only unhides active memberships, so it cannot restore this user. Gate the leave/lifecycle controls out for DMs or wire the details page to the DM-hide operation instead, otherwise a confirmed tap permanently removes access to the conversation.
Useful? React with 👍 / 👎.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE at exact head bf1a89d5b4a59cc7af1cd3c72344e31b1e47f619 (base e982f70fba29cdaa9a8378f118a0e498537bd8db).
No author-actionable defect was established across the systems/integration and adversarial product/UI review.
What was verified
- The Flutter→UIKit navigation payload remains symmetric: Flutter emits background/theme, title, presence, actions, and
plain; Swift decodes and reapplies those fields (mobile/lib/shared/widgets/ios_navigation_bar.dart:194-213,mobile/ios/Runner/IosNavigationBar.swift:264-340). - Controller/listener ownership is bounded on both sides: native handlers are weak and child controllers/views are removed; Flutter removes method-channel and scroll listeners (
IosNavigationBar.swift:476-480,ios_navigation_bar.dart:216-274). - Blur depth handles compact and reversed timelines via axis-aware
extentAfter/extentBefore, while native material updates on configure and scroll and clamps negative offsets (frosted_scaffold.dart:131-170,IosNavigationBar.swift:283-292,377-395). - Search retains the same
TextField/FocusNode, fences post-frame focus with mounted/editing state, removes listeners, reserves a separate Cancel hit target, and honors reduced motion (search_page.dart:60-81,115-156,170-217,235-240,330-405). - Conversation titles route through the shared details flow; native title sizing/truncation, activation, presence layout, and Dynamic Type are covered by production-bound XCTest assertions (
channel_detail_page.dart:585-632,IosNavigationBar.swift:24-134,mobile/ios/RunnerTests/RunnerTests.swift:11-148). - DM details use the counterpart identity/avatar for 1:1 conversations and suppress group-only controls while preserving them for group DMs (
channel_details_page.dart:121-147,319-421). Profile descriptions remain in the scrollable sheet directly below the centered name (user_profile_sheet.dart:196-240). - No persistence/event/schema format changed.
git diff --checkwas clean, the reviewed tree was clean, DCO passed, and one reviewer ranjust mobile-checksuccessfully (format over 621 files; analyzer clean). - Required exact-head CI finished with no pending or failed checks:
Clients / Mobile,Clients / Results,Mobile Swift Domain / Mobile Swift,Mobile Swift Domain / Results, DCO, security review, Semgrep, and zizmor are green. GitHub identity was verified asjedwards27; the PR author isklopez4212.
Confidence gaps (not author rework)
- Runner XCTest execution is not demonstrated by CI. The green Mobile Swift job log shows the BuzzPushKit package suite (78 tests) but no
RunnerTestsexecution; the added production-bound XCTest assertions therefore remain unexecuted in available evidence. Author action: none. Verification owner: CI/release gate or reviewer tooling. - Reviewer-side Flutter widget/simulator runs were blocked by this host’s unaccepted Xcode license (
xcruncould not resolve an SDK; native-assets setup failed). Author action: none. Verification owner: reviewer/tooling. - Native visual journeys were not directly observed across narrow widths, light/dark themes, accessibility text sizes, search keyboard/Cancel, title activation, iOS 26 shared-background behavior, and newest→scrolled blur. The reported physical-iPhone Release install/launch is useful but does not cover that interaction matrix. Author action: none. Verification owner: reviewer/tooling or release validation.
Residual risk is confined to unobserved native visual/integration behavior, not a demonstrated regression. Any new head invalidates this approval until its delta and affected gates are reviewed.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: e982f70fba29cdaa9a8378f118a0e498537bd8db..bf1a89d5b4a59cc7af1cd3c72344e31b1e47f619 (exact head bf1a89d5b4a59cc7af1cd3c72344e31b1e47f619)
Risk: medium — visible iOS navigation, UIKit/Flutter lifecycle, search focus, blur/theme state, Dynamic Type, and conversation-details behavior.
Behavior/contracts traced: Flutter→UIKit navigation payload symmetry; native child-controller/view ownership; title tap/truncation/presence and accessibility activation; axis-aware blur for reversed timelines; search focus/Cancel/reduced-motion lifecycle; DM versus group details/actions; profile layout; persistence/schema boundaries; exact-head CI.
Findings: no unresolved blocking or non-blocking code defect. Both independent review lanes found the renderer/native contract coherent. Flutter sends the live background/title/presence/actions/plain state and Swift reapplies material, tint, title, and actions. Native and Flutter listeners/controllers are cleaned up. Reversed timelines use axis-aware scroll extent; search retains one field/focus node, fences post-frame work, and honors reduced motion. DM/group detail routing and actions remain semantically separated. No relay, identity, storage schema, or persistence contract changed.
Author action: none.
Verification owner: CI/release gate for the still-running exact-head Clients / Mobile job; reviewer/tooling for additional native visual interaction evidence.
Validation at matching exact head:
just mobile-check— PASS; format across 621 files and analyzer reported no issues.git diff --check— PASS; review worktree clean.Mobile Swift Domain / Mobile Swift— PASS, including build, release build, test, complete iOS simulator app, and unsigned release build.Mobile Swift Domain / Results, security review, Semgrep, zizmor, DCO — PASS.Clients / Mobile: format, analyze, full tests, no-push test, and recipe-argument test all passed; Android debug APK build remained in progress at submission.
Manual/native evidence: the author reports the Release iOS build installed and launched on a physical iPhone. Reviewers did not independently capture the title-tap/truncation, narrow-width, light/dark, AX text-size, search keyboard/Cancel, or newest→scrolled blur journeys. Local widget/native execution was blocked by this host’s unaccepted Xcode license. Those are confidence gaps, not author-actionable defects.
Residual risk: exact visual behavior across iOS versions, Dynamic Type extremes, blur tint, and focus animation was not independently witnessed. A PR-caused failure in the pending required mobile gate should reopen this approval. Any new head invalidates it.
Fix iOS navigation title taps and truncation, theme-aware blur (including chats opened at the newest message), and the search focus animation with a plain, correctly spaced Cancel button.
Simplify DM details and header actions, move profile descriptions below names, and remove redundant Settings/Add Community titles and unnecessary “See all” links.
Validation: mobile analysis, formatting, file-size checks, and the full mobile test suite passed in pre-push hooks; Release iOS build installed and launched on a physical iPhone. Full
just ciwas stopped during unrelated desktop compilation due to low disk space. Native XCTest additions have not been run locally.