Skip to content

Use correct channel icons in channel chat screen#444

Merged
zjs81 merged 1 commit into
zjs81:devfrom
sethoscope:fix-channel-chat-icon
May 12, 2026
Merged

Use correct channel icons in channel chat screen#444
zjs81 merged 1 commit into
zjs81:devfrom
sethoscope:fix-channel-chat-icon

Conversation

@sethoscope

Copy link
Copy Markdown
Contributor

At the top of the channel chat screen is an icon, indicating the channel type.

Previously, the public icon was used correctly, but the hashtag icon was used for all other types.

Now, consistent with the channels screen, we use the lock icon for private channels, and the composite icons for community public & community hashtag types.

Closes issue #432.

@sethoscope

Copy link
Copy Markdown
Contributor Author

Just a note... Originally I only thought to fix the private channel icon, and that was a 2-line change. But as I dug in more, I finally understood what community channels are and how they work, and those icons were wrong too.

Fixing that was much more involved, because it's not easy to tell whether a channel is part of a community. I ended up duplicating some things from channels_screen.dart. I tried a few times to refactor that to make it more reusable, but I didn't come up with anything I liked, and I decided this was better for now.

@zjs81 zjs81 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the fix — the classification matches channels_screen.dart and renders correctly. A few things to address before merge:

  1. Dedupe the classification with channels_screen.dart. The same 5-way logic now lives in two places (channels_screen._getCommunityForChannel/_isCommunityPublicChannel and the new getChannelType here). Please pull this into a shared helper in this PR — e.g. a static ChannelType.classify(Channel channel, Iterable<Community> communities) next to the new enum in lib/models/channel.dart — and refactor channels_screen.dart to use it too. The enum is already in the model, so the logic should live there as well; otherwise the two screens will drift.
  2. Restore size: 20 on the AppBar icon — current code falls back to the default (24), which is a visible regression versus the previous Icon(..., size: 20).
  3. Replace default: with explicit case ChannelType.private: so the Dart 3 switch is exhaustive and the analyzer warns when a new variant is added.
  4. Prefix new helpers with _ (_getChannelType, _channelIcon) — they're state-internal and the rest of the file uses that convention.
  5. First-frame flicker_loadCommunities() is fired async after the first frame, so community channels render as Icons.lock (private) until it resolves. Worth a placeholder or a synchronous cached read; see inline.
  6. Minor: the // Use a stack in case we want to add community annotation comment is misleading — the Stack is already doing that.

No test changes needed.

Future<void> _loadCommunities() async {
final connector = context.read<MeshCoreConnector>();
_communityStore.setPublicKeyHex = connector.selfPublicKeyHex;
final communities = await _communityStore.loadCommunities();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Fire-and-forget async load means a community channel renders as Icons.lock (the private default) on the first frame, then swaps once communities load. On slower devices this flickers visibly.

Two options:

  • Render a neutral placeholder (or hide the icon) until _communities has loaded at least once, or
  • Make CommunityStore.loadCommunities() return a synchronously-cached list when one is available.

Also note: _communities never refreshes while this screen is open, so edits made elsewhere won't be reflected until the screen is recreated — acceptable for now but worth a one-line comment if you don't want to wire it to the store.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'm new to Flutter & Dart, so I'm still getting up to speed on a lot of this.

Does channels_screen have the same flicker problem? It looks like the only difference between the two is that here _loadCommunities() is run from SchedulerBinding.instance.addPostFrameCallback and in channels_list it's from WidgetsBinding.

I regret to say I don't understand the second option.

Re the refresh note: I think I get it. We only load communities the first time, and if we navigate back here after changing them, the list will be stale? 20 minutes of reading about this stuff has mainly taught me that there are good idiomatic ways of managing this in Flutter, but I don't yet know what they are.

I force pushed what I have so far, which fixes all the issues you pointed out except this one.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'll do some testing I've noticed this before on my phone when I had lots of channels and I put them in custom positions.

Comment thread lib/screens/channel_chat_screen.dart Outdated
Comment on lines +212 to +233
ChannelType getChannelType(Channel channel) {
// Go through the communities and their channels and see whether
// this channel matches one of them.
for (final community in _communities) {
final publicPsk = community.deriveCommunityPublicPsk();
if (channel.pskHex == Channel.formatPskHex(publicPsk)) {
return ChannelType.communityPublic;
}
for (final hashtag in community.hashtagChannels) {
final hashtagPsk = community.deriveCommunityHashtagPsk(hashtag);
if (channel.pskHex == Channel.formatPskHex(hashtagPsk)) {
return ChannelType.communityHashtag;
}
}
}
if (channel.isPublicChannel) {
return ChannelType.public;
} else if (channel.name.startsWith('#')) {
return ChannelType.hashtag;
}
return ChannelType.private;
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please dedupe this with channels_screen.dart in this PR rather than as a follow-up.

channels_screen.dart:378-409 already classifies channels with the same logic via _getCommunityForChannel / _isCommunityPublicChannel. With the new ChannelType enum living in the model, the natural home for this is a static helper alongside it, e.g.:

// lib/models/channel.dart
enum ChannelType { public, private, hashtag, communityPublic, communityHashtag }

extension ChannelTypeClassify on Channel {
  ChannelType classify(Iterable<Community> communities) {
    for (final community in communities) {
      if (pskHex == Channel.formatPskHex(community.deriveCommunityPublicPsk())) {
        return ChannelType.communityPublic;
      }
      for (final hashtag in community.hashtagChannels) {
        if (pskHex == Channel.formatPskHex(community.deriveCommunityHashtagPsk(hashtag))) {
          return ChannelType.communityHashtag;
        }
      }
    }
    if (isPublicChannel) return ChannelType.public;
    if (name.startsWith('#')) return ChannelType.hashtag;
    return ChannelType.private;
  }
}

Then channel_chat_screen.dart calls widget.channel.classify(_communities) and channels_screen.dart is refactored to use the same call instead of its ad-hoc _getCommunityForChannel / _isCommunityPublicChannel. That keeps the two screens from drifting as new channel types are added.

Also rename to _getChannelType / drop entirely once this is extracted.

Comment thread lib/screens/channel_chat_screen.dart
Comment thread lib/screens/channel_chat_screen.dart Outdated
children: [
Padding(
padding: const EdgeInsets.symmetric(vertical: 3),
child: Icon(icon),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Visible regression: the previous AppBar icon was Icon(..., size: 20). This Icon(icon) falls back to the AppBar default (24), so the title-row icon is now noticeably larger than before.

Suggested change
child: Icon(icon),
child: Icon(icon, size: 20),

If you keep the size, also scale the community badge Container (currently 14×14) proportionally so the annotation doesn't overflow the smaller icon.

Comment thread lib/screens/channel_chat_screen.dart Outdated
icon = Icons.lock;
}
return Stack(
// Use a stack in case we want to add community annotation

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: this comment is misleading — the Stack is already used for the community-badge annotation a few lines below, not "in case we want to add" one. Either remove or reword to describe what it actually does.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I meant "in case we want to add one" because the inclusion of the badge is conditional, not for future development. But the comment clearly does more harm than good, so out it goes.

@sethoscope sethoscope force-pushed the fix-channel-chat-icon branch from a1e8a94 to 0521df5 Compare May 11, 2026 21:44
At the top of the channel chat screen is an icon, indicating the
channel type.

Previously, the public icon was used correctly, but the
hashtag icon was used for all other types.

Now, consistent with the channels screen, we use the lock icon for
private channels, and the composite icons for community public &
community hashtag types.

The fix for private channels was trivial, as we can identify hashtag
channels by their name. Finding out whether a channel belongs to a
community is much more involved. All the hard-working code was copied
from channels_screen.dart. (I tried refactoring to reduce duplication,
but my results were complex and not worth it.)

Closes zjs81#432
@sethoscope sethoscope force-pushed the fix-channel-chat-icon branch from 0521df5 to 2763d83 Compare May 12, 2026 04:14
@sethoscope

Copy link
Copy Markdown
Contributor Author

@zjs81
Ok, I think I've fixed all the problems you pointed out, other than refreshing the community list in channel_chat_screen after it's updated elsewhere.

I didn't observe the flickering problem, so I'm taking my fix on faith. Probably I should get set up with some virtual test devices.

@zjs81

zjs81 commented May 12, 2026

Copy link
Copy Markdown
Owner

Thanks I'll review it again.

@zjs81 zjs81 merged commit 8892823 into zjs81:dev May 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants