Skip to content

fix(channels): unify the messaging default-channel selector into one grid#4109

Merged
graycyrus merged 1 commit into
tinyhumansai:mainfrom
graycyrus:fix/messaging-default-channel-ui
Jun 25, 2026
Merged

fix(channels): unify the messaging default-channel selector into one grid#4109
graycyrus merged 1 commit into
tinyhumansai:mainfrom
graycyrus:fix/messaging-default-channel-ui

Conversation

@graycyrus

@graycyrus graycyrus commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

The Messaging settings panel rendered the channel list twice — a grid of connect/configure tiles plus a separate "Default Messaging Channel" button list, both labeled the same — so users couldn't tell which one actually set the default (the two even covered different channel sets).

This collapses them into a single horizontal-card grid where each tile shows connection status, opens setup on click, and owns the default selection:

  • Icon on the left; name → status → default control stacked on the right.
  • "Set as default" appears only on connected channels; the current default shows a ✓ Default badge plus a primary ring on the tile.
  • Connected and not-connected channels render as two separate rows (no divider/label between them).
  • Web stays selectable (it's always available / built-in).

Before → After

  • Before: a connect-tile grid and a duplicate "DEFAULT MESSAGING CHANNEL" button list, same labels, ambiguous.
  • After: one grid; the tile is the single source of truth for both connecting and choosing the default.

Changes

  • app/src/pages/Skills.tsxChannelTile redesigned (horizontal layout, connected-only "Set as default", default badge + ring); the channels tab now renders connected/not-connected as two grids; removed the duplicate selector list.
  • i18n: added channels.setAsDefault / channels.defaultBadge across all 14 locales (real translations).
  • Tests: Skills.channels-grid.test.tsx updated + new cases (connected-only control, connected-first grouping). The e2e/playwright "switch default channel" specs now switch via the always-connected Web tile (disconnected channels no longer expose the control), pre-setting Telegram as default so Web becomes a connected, non-default tile.

Test plan

  • pnpm typecheck passes
  • pnpm lint passes (0 errors)
  • pnpm format applied
  • pnpm i18n:check passes (full parity across 14 locales)
  • Unit: Skills.channels-grid.test.tsx 7/7; full pages/__tests__ suite 225/225
  • E2E (WDIO + Playwright) "switch default channel" specs updated for the new gating — validated by CI
  • Launched the desktop app locally and verified the new layout on Connections → Messaging

…grid

The Messaging settings panel rendered the channel list twice — a grid of
connect/configure tiles plus a separate "Default Messaging Channel" button
list, both labeled the same — so it was unclear which one set the default.

Collapse them into a single horizontal-card grid where each tile shows
connection status, opens setup on click, and owns the default selection:
- icon on the left; name -> status -> default control stacked on the right
- "Set as default" appears only on connected channels; the current default
  shows a Default badge plus a primary ring on the tile
- connected and not-connected channels render as two separate rows
- Web stays selectable (always available)

Add channels.setAsDefault / channels.defaultBadge i18n keys across all
locales. Update channel-grid unit tests and the e2e/playwright specs that
switched the default (now via the always-connected Web tile).
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Channels default controls were added to each channel tile, the page now separates connected and disconnected grids, new locale strings were introduced, and unit plus end-to-end tests were updated for the default-channel flow.

Changes

Channels default selection flow

Layer / File(s) Summary
Locale strings
app/src/lib/i18n/*.ts
Adds channels.setAsDefault and channels.defaultBadge entries across the locale message maps.
ChannelTile default action
app/src/pages/Skills.tsx
ChannelTile gains default-state props and renders either a default badge or a set-as-default control beneath the configure button.
Channels grid wiring
app/src/pages/Skills.tsx
The Channels page splits connected and disconnected grids, treats web as connected, and passes default-selection state and handlers into each tile.
Channels grid tests
app/src/pages/__tests__/Skills.channels-grid.test.tsx
The grid tests add a Web channel mock, stub updatePreferences, and verify ordering, default control visibility, default switching, and container styling.
Default-channel E2E flows
app/test/e2e/specs/settings-*.spec.ts, app/test/playwright/specs/settings-*.spec.ts
The E2E and Playwright specs seed telegram through RPC, read persisted default state, and switch the default to web through the channel selector.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

  • tinyhumansai/openhuman#3649: Both PRs touch the default-messaging channel selection in app/src/pages/Skills.tsx and update E2E flows that select the default channel.

Suggested labels

working

Suggested reviewers

  • oxoxDev

Poem

A bunny hopped through channels bright,
and found the default tucked just right.
A badge said “Default” — neat and true,
while Web hopped up to lead the queue.
🐇🥕 The tests all squeaked, “Hooray!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: consolidating the messaging default-channel selector into a single grid.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands.

@graycyrus

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label Jun 25, 2026
@graycyrus graycyrus marked this pull request as ready for review June 25, 2026 10:36
@graycyrus graycyrus requested a review from a team June 25, 2026 10:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 917f3f1171

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread app/src/pages/Skills.tsx
// into one place. "Set as default" only appears for channels you can actually
// route through (connected); the default badge still shows on whichever
// channel is persisted as default, connected or not.
const showDefaultControl = isDefault || isConnected;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep backend-only channels selectable as defaults

In the macOS iMessage path, core still advertises imessage from channels_list (src/openhuman/channels/controllers/definitions.rs:310-318), but the frontend ChannelType/KNOWN_CHANNEL_TYPES omit it (app/src/types/channels.ts:1-15), and useChannelDefinitions skips status entries that fail that guard (app/src/hooks/useChannelDefinitions.ts:92-97). That means even a configured iMessage channel reaches this line with isConnected === false; because it is normally not already the default, the unified grid no longer renders any channel-select-imessage control. The removed selector rendered a button for every channelDefs entry, so this regresses users who want to make configured iMessage the default.

Useful? React with 👍 / 👎.

@graycyrus graycyrus merged commit fc563eb into tinyhumansai:main Jun 25, 2026
45 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant