Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors the server top bar to a ConsumerWidget with breakpoint-aware leading content and aggregated connection status; adds a Github participant ID; guards avatar initial extraction; updates fl_lib git ref and removes a local dependency override; deletes a submodule-commit line in the wiki. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as TopBar (ConsumerWidget)
participant Breakpoints as BreakpointService
participant Riverpod as Providers
User->>UI: Open Server Tab
UI->>Breakpoints: isMobile?
Breakpoints-->>UI: true/false
UI->>Riverpod: watch(serverOrder)
Riverpod-->>UI: [serverIds]
loop per serverId
UI->>Riverpod: watch(conn[serverId])
Riverpod-->>UI: conn state
end
alt Desktop
UI-->>User: Render leading: "X/Y connected" (aggregated)
else Mobile
UI-->>User: Render leading: settings button
end
UI-->>User: Render TagSwitcher and trailing controls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/view/page/server/tab/top_bar.dart (1)
56-65: TagSwitcher still renders on desktop—breaks PR objective.Even in the desktop branch we still place
TagSwitcher(...).expanded()in the row, so the tag switcher remains visible. That contradicts “no Tag Switcher on desktop” and keeps the old UI. Please gate the widget behind theisMobilecheck (e.g., only include it inchildrenwhenisMobileis true, otherwise insert aSpacer()or nothing).A minimal adjustment could look like:
- children: [ - leading, - SizedBox(width: isMobile ? 30 : 16), - TagSwitcher(...).expanded(), - ], + children: [ + leading, + if (isMobile) ...[ + const SizedBox(width: 30), + TagSwitcher( + tags: tags, + onTagChanged: onTagChanged, + initTag: initTag, + singleLine: true, + reversed: true, + ).expanded(), + ], + ],Feel free to adjust spacing for the desktop branch as needed.
🧹 Nitpick comments (1)
lib/view/page/setting/seq/srv_seq.dart (1)
116-128: Handle full grapheme clusters for initials.
String.fromCharCode(spi.name.runes.first)chops multi-code-point graphemes (e.g., emoji flags). Prefer thecharactersextension (bundled with Flutter) to extract the first visible character cleanly.You can update the snippet like:
- final String name; - if (spi.name.isNotEmpty) { - name = String.fromCharCode(spi.name.runes.first); - } else { - name = '?'; - } + final name = spi.name.characters.isNotEmpty ? spi.name.characters.first : '?';Make sure to import
package:characters/characters.dart; it’s part of Flutter’s SDK, so no extra dependency needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
flutter_server_box.wiki(0 hunks)lib/data/res/github_id.dart(1 hunks)lib/view/page/server/tab/top_bar.dart(1 hunks)lib/view/page/setting/seq/srv_seq.dart(1 hunks)pubspec.yaml(2 hunks)
💤 Files with no reviewable changes (1)
- flutter_server_box.wiki
🧰 Additional context used
📓 Path-based instructions (2)
pubspec.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hive_ce instead of hive for Hive integration
Files:
pubspec.yaml
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/setting/seq/srv_seq.dartlib/view/page/server/tab/top_bar.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check
Fixes #931
Summary by CodeRabbit
New Features
Bug Fixes
Chores