Code quality and UI improvements, and i18n update#1215
Merged
Conversation
Replace SizedBox.shrink() placeholder routes with _EmptyRoute that auto-pops via post-frame callback. Previously, invalid arguments or deleted servers would push invisible routes that persisted in the semantics tree, triggering AXTree update failures.
Convert _tabMap.entries to a list once in PageView.builder and _syncVisibleTabs instead of calling .keys.elementAt(idx) and .values.elementAt(i) in loops, which are O(n) per iteration.
HitTestBehavior.opaque marked the entire server list body as a semantics tap target and could interfere with event routing to child widgets. Use translucent to preserve tap-for-unfocus behavior without swallowing events.
Move GestureDetector from wrapping the entire Scaffold to only wrapping the form body. This prevents assigning a tap semantics role to the whole page including the app bar and FAB.
Standardize all server sub-pages to use TwoLineText with feature
name on top and server name on bottom:
- systemd.dart: Text('Systemd') -> TwoLineText(up: 'Systemd', down: spi.name)
- port_forward.dart: Text(portForward+Beta) -> TwoLineText(up: portForward+Beta, down: spi.name)
- process.dart: swap TwoLineText up/down to match consistent layout
- container.dart: Remove unused _textController field and dispose call - container.dart: Remove unreachable null-coalescing fallback in _buildLoading - container.dart: Remove ineffective SizedBox(height: 3) inside a Row - container.dart: Remove always-false enum values.length checks - container.dart: Simplify redundant async/await in FAB onPressed - view.dart: Remove unused import of cmd_types.dart - misc.dart: Remove unused _NetSortType getters (isDevice, isIn, isOut) - view.dart: Remove unreachable null-coalescing on si.summary
Wrap SSH command execution in try/catch to handle connection drops and timeouts gracefully. Add mounted checks before UI updates. Only call setState after successful data parse to avoid unnecessary rebuilds when data hasn't changed.
…ver detail - _buildMemView: Return null when ss.mem.total is 0 to prevent NaN/Infinity - _buildProgress: Use clamp() instead of mutating the method parameter
- systemd.dart: Change _showConfirmDialog from void to Future<void> - port_forward.dart: Change _onSave from void to Future<void> - port_forward.dart: Use context.pop() instead of Navigator.of(context).pop()
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates several Flutter pages with lifecycle, rendering, gesture, and navigation adjustments. It also adds new localized strings for the port-forward beta title and systemd labels across multiple ARB files, and updates page titles to use two-line app bar layouts in the port-forward, process, and systemd screens. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/view/page/port_forward.dart`:
- Around line 74-76: The page title in TwoLineText still hardcodes the “(Beta)”
suffix, so move the entire label into localization instead of concatenating it
in port_forward.dart. Update the title construction to use a localized string
from libL10n/l10n (prefer libL10n) that already includes the beta marker, and
adjust the relevant localization entries so the Port Forward title is fully
translated without any raw English suffix.
In `@lib/view/page/systemd.dart`:
- Around line 33-34: The app-bar title in SystemdPage is hardcoded as
user-facing text, so replace the literal in the title assignment with a
localized string from libL10n/l10n. Update the Systemd page widget to use the
existing localization accessor instead of the string constant, keeping the
dynamic subtitle from widget.args.spi.name unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b6316419-f7f4-42b4-a92d-5a010385083b
📒 Files selected for processing (10)
lib/view/page/container/container.dartlib/view/page/port_forward.dartlib/view/page/process.dartlib/view/page/server/detail/misc.dartlib/view/page/server/detail/view.dartlib/view/page/server/edit/edit.dartlib/view/page/server/tab/tab.dartlib/view/page/ssh/page/page.dartlib/view/page/ssh/tab.dartlib/view/page/systemd.dart
💤 Files with no reviewable changes (1)
- lib/view/page/server/detail/misc.dart
- port_forward.dart: Replace inline ' (Beta)' with localized libL10n.portForwardBetaTitle - systemd.dart: Replace hardcoded 'Systemd' with localized libL10n.systemd - Add portForwardBetaTitle and systemd strings to all 15 ARB files - Regenerate l10n with flutter gen-l10n
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/l10n/app_de.arb`:
- Line 257: The new page title is still hardcoded in English for German users;
update the German localization entry used by Port Forward. Adjust the
translation for portForwardBetaTitle in the app localization resources so the
AppBar in PortForwardPage shows a German title instead of the English string.
In `@lib/l10n/app_fr.arb`:
- Line 257: The new Port Forward page title is still hardcoded in English in the
French localization bundle. Update the existing app bar string entry for port
forwarding in app_fr.arb so it uses a proper French translation, and make sure
the key used by the Port Forward title remains consistent with the rest of the
localization files.
In `@lib/l10n/app_ko.arb`:
- Line 262: The new Port Forward page title is still hardcoded in English, so
update the localization entry in app_ko.arb for portForwardBetaTitle to a proper
Korean translation. Keep the key unchanged and make sure the translated value is
used by the Port Forward app bar so Korean users see localized text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 22b9791b-92dc-4030-9a56-8281b7c264e4
⛔ Files ignored due to path filters (15)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_it.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ko.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**
📒 Files selected for processing (17)
lib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_id.arblib/l10n/app_it.arblib/l10n/app_ja.arblib/l10n/app_ko.arblib/l10n/app_nl.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/l10n/app_tr.arblib/l10n/app_uk.arblib/l10n/app_zh.arblib/l10n/app_zh_tw.arblib/view/page/port_forward.dartlib/view/page/systemd.dart
✅ Files skipped from review due to trivial changes (12)
- lib/l10n/app_pt.arb
- lib/l10n/app_it.arb
- lib/l10n/app_es.arb
- lib/l10n/app_ru.arb
- lib/l10n/app_tr.arb
- lib/l10n/app_uk.arb
- lib/l10n/app_en.arb
- lib/l10n/app_zh.arb
- lib/l10n/app_nl.arb
- lib/l10n/app_ja.arb
- lib/l10n/app_zh_tw.arb
- lib/l10n/app_id.arb
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/view/page/systemd.dart
- lib/view/page/port_forward.dart
- German: 'Port Forwarding (Beta)' - French: 'Transfert de port (Beta)' - Korean: '포트 포워딩 (Beta)' Regenerate l10n files with flutter gen-l10n.
GT-610
changed the title
- port_forward.dart: Use l10n (local) instead of libL10n (fl_lib) for portForwardBetaTitle since the string is defined in local ARB files - systemd.dart: Use l10n (local) instead of libL10n for systemd, and add missing locale.dart import - ssh/tab.dart: Remove unreachable null-coalescing fallback on entries[idx].value.page (page is non-nullable Widget)
The cmd_types.dart import provides the StatusCmdTypeX extension that defines the i18n getter used at view.dart:206. It was incorrectly removed as 'unused' in a prior commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit