Unify mobile channel details - #6113
Conversation
Signed-off-by: kenny lopez <klopez4212@gmail.com>
|
🤖 Fresh Pixel snapshots from the exact PR build. Channel headerThe channel name and collective member count form the new details entry point.
Combined channel detailsIdentity, quick actions, and the compact five-member preview live on one page.
Manage channelEditing keeps the focused name, description, and canvas controls.
|
Signed-off-by: kenny lopez <klopez4212@gmail.com>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
The redesigned management surfaces need accessible names and state before this lands:
mobile/lib/features/channels/manage_channel_sheet.dart:112-130,287-308gives the pre-populated Name and Description fields onlyhintText. Because both fields already contain values, assistive technology does not get a persistent field name and users have to infer which value they are editing. The new test atmobile/test/features/channels/channel_detail_page_test.dart:1360-1368explicitly expectslabelText == nullbut does not assert semantics. Please preserve the outline-only design if desired, but add reliable accessible names (for example, non-duplicatingSemanticslabels) and test both pre-populated fields through the semantics tree.mobile/lib/features/channels/add_members_sheet.dart:161-191represents selection solely by swapping the trailing plus/check icon. The tappable result row exposes no selected/toggled semantic state, so VoiceOver/TalkBack users cannot reliably determine whether that person is selected. Please expose the row as a button/control with its current selected state and cover selection and deselection semantics.
These are regressions in core channel-administration workflows, not visual polish.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
One additional blocking authorization defect surfaced during the consolidated review:
ChannelActionsSheet exposes Manage channel to every non-DM (mobile/lib/features/channels/channel_actions_sheet.dart:208-224), while this change weakens ManageChannelSheet.canEditCanvas from channel.isMember && !channel.isArchived to only !channel.isArchived (mobile/lib/features/channels/manage_channel_sheet.dart:50-54). A nonmember browsing an open channel can therefore open this legacy path and submit a kind:40100 canvas replacement. The relay's generic authorization admits nonmembers to open channels (crates/buzz-relay/src/handlers/ingest.rs:643-679), and canvas is not one of the kinds routed through a stricter per-kind admin validator (:2290-2304). Because canvas content feeds agent context, this permits drive-by modification of operational instructions.
Restore an explicit membership/moderation capability gate for canvas editing, and test the channel-tile → Manage path for an open-channel nonmember. The same path currently presents name/description editors to users whom kind:9002 will reject; gate those controls with the resolved management capability rather than offering a guaranteed failure.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7c776e9b6
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7c776e9b6
ℹ️ 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".
Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19ede787df
ℹ️ 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".
Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d34728722c
ℹ️ 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".
Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6810faf909
ℹ️ 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".
Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e13e9cfd99
ℹ️ 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".
Signed-off-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz> Co-authored-by: Fast Fizz <2df81cb51f05a9d5387ef24d7b9ecb8fcdfcd1c70ffabc67061c9596e1b5b1c4@buzz.block.builderlab.xyz>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent, commenting via Wes’s GitHub account.
Consolidated review — changes requested
Reviewed exact head a8e6aee0191995136a6c446afabc19f6eb0a667c against base f956e6fe06a76e50cbd8fba1a162482e752e7f1a. The combined review is not merge-ready. Three material workflow/state-boundary defects remain:
-
Small channels lose member administration.
ChannelDetailsPageonly renders See all whenmembers.length > 5(mobile/lib/features/channels/channel_details_page.dart:382-390), while this PR removes the non-DM app-bar Members button (mobile/lib/features/channels/channel_detail_page.dart:333-361). Preview rows open profiles, notMembersSheet(channel_details_page.dart:371-381,636-656). Owners/admins in channels with five or fewer members therefore cannot reach the role-change/remove controls that exist only inMembersSheet(members_sheet.dart:225-275,316-360). The test atchannel_detail_page_test.dart:908-947currently locks in the inaccessible state. Retain a route to the complete member-management surface and add a biting ≤5-member role/remove navigation regression. -
Add members is actionable before the roster is authoritative. For an open channel,
canAddMembers(null)is true, so the row is shown whilechannelMembersProvideris loading or failed (channel_details_page.dart:65-66,92-94,175-193,333-355;channel.dart:86-95). The sheet then receives an emptyexistingPubkeys, can offer an existing owner/admin, and submits explicitrole=member(add_members_sheet.dart:44-58,78-83;channel_management_provider.dart:693-724). The relay treats changing an active member’s role as a real privileged role change (crates/buzz-db/src/channel.rs:456-503), so this race can accidentally demote an existing member. A temporary widget regression requiring the Add row to remain absent during a pending roster failed causally. Keep Add members hidden/disabled until a successful authoritative roster load, and cover pending/error states. -
A stale community-scoped action object can publish channel mutations after the user switches communities.
channelActionsProvidercaptures the old relay identity and supplies anisCommunityValidfence (channel_management_provider.dart:971-987), but onlyaddMembersinvokes it (:693-735).updateChannelsubmits kind9002without checking the fence (:768-783); leave/archive/unarchive/delete/canvas and role/remove methods have the same missing boundary check. I retained aChannelActions, switchedrelayConfigProvider, attached the new community socket, then invokedupdateChannel: the regression expected no event but recorded a signed kind9002forchannel-1. Evidence is local review artifact.scratch/pr6113-carl/stale-update-regression.log; the temporary test was removed and the worktree restored clean. Apply the community-validity fence before every scoped publish (and where needed after awaited boundaries before cache invalidation), with regressions for metadata plus destructive/member lifecycle actions.
The accessibility semantics, lifecycle confirmation/success ordering, partial-add invalidation, metadata/canvas refresh paths, and supplied screenshots were also reviewed without an additional blocker. Those repaired paths do not override the demonstrated failures above.
Exact-head evidence
- PASS —
just mobile-check(format/analyze/size gate). - PASS — full
just mobile-test: 1,481 tests. - PASS — all reported required GitHub checks, including Mobile, at
a8e6aee0. - PASS — post-probe local HEAD = live PR head; worktree clean.
- FAIL (causal probe) — stale
ChannelActions.updateChannelpublished kind9002after a community switch. - NOT RUN independently — real Pixel/iOS interaction and assistive-technology journey; the author reports Pixel 10 interaction review.
Green CI does not exercise the three states above. Please address them and add the risk-shaped regressions before merge.
3 participants
Summary
Testing
just mobile-checkjust mobile-test— 1,477 testsSnapshots
Channel header
Combined channel details
Manage channel