Skip to content

refacto: settings & router code hygiene#2945

Open
9clg6 wants to merge 2 commits intomainfrom
refacto/TW-2991-settings-router-code-hygiene
Open

refacto: settings & router code hygiene#2945
9clg6 wants to merge 2 commits intomainfrom
refacto/TW-2991-settings-router-code-hygiene

Conversation

@9clg6
Copy link
Collaborator

@9clg6 9clg6 commented Mar 19, 2026

Summary

Split from #2914 — this PR contains only the refactoring (no feature changes).

  • Reorganize imports (alphabetical, grouped by package)
  • Extract repeated L10n.of(context)! into local l10n variables
  • Extract repeated Theme.of(context).colorScheme into local variables
  • Extract repeated LinagoraRefColors.material().tertiary[30] into local variables
  • Use Dart 3 enum shorthand (.center, .number, .ellipsis, etc.)
  • Add mounted checks after async operations
  • Replace Matrix.of(context).client with existing client getter
  • Replace Clipboard.setData with TwakeClipboard.instance.copyText
  • Simplify conditional widget logic (trailingWidget)
  • Remove unused settings_profile_ui_state.dart
  • Clean up showBootstrapDialog signature (remove unused BuildContext param)
  • Centralize route path definitions

Related PR (feature): #2944

Summary by CodeRabbit

  • Refactor
    • Centralized and standardized in-app navigation for settings, profile (incl. QR), chat (emotes), security, notifications, style, language, devices, and add-account flows for consistent routing.
    • Reused cached localization and theme values across settings screens to reduce redundant lookups and improve UI responsiveness.
    • Streamlined settings/profile UI internals (mobile & web) and emotes/settings lists for more stable rendering and easier navigation.

@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

Walkthrough

This PR adds many new route segment and full-path constants to AppRoutePaths and refactors route construction to compose paths from those segments. go_router usage is updated to reference the new constants instead of hard-coded strings. Multiple pages and widgets across settings, profile, chat, and account flows were changed to use these constants; numerous files also cache repeated context-dependent values (L10n, Theme, colors), apply shorthand Flutter syntax, extract helper widgets, and remove an unused SettingsProfileUIState class.

Possibly related PRs

Suggested reviewers

  • dab246
  • hoangdat
  • nqhhdev
  • tddang-linagora
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is partially complete but does not follow the provided template structure. Critical sections like Ticket, Root cause, Solution, Impact, Test recommendations, and Pre-merge are missing or not properly formatted. Restructure the description to match the template: add Ticket/Related issue, Root cause (if applicable), Solution section with clear details, Impact description, Test recommendations, Pre-merge checklist, and Resolved section with platform screenshots.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refacto: settings & router code hygiene' accurately describes the main focus of the PR as a refactoring effort targeting code cleanliness in settings and router modules.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refacto/TW-2991-settings-router-code-hygiene

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Contributor

This PR has been deployed to https://linagora.github.io/twake-on-matrix/2945

@9clg6
Copy link
Collaborator Author

9clg6 commented Mar 19, 2026

This PR is similar to the one proposing a complete refactoring of the Router, but here, I simply continued what was already in place: the use of AppRoutePaths. So this isn't a refactoring, but rather a code cleanup.

@9clg6 9clg6 marked this pull request as ready for review March 19, 2026 10:34
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
lib/pages/settings_dashboard/settings_emotes/settings_emotes.dart (1)

179-216: ⚠️ Potential issue | 🟠 Major

Guard the post-save state update with mounted.

addImageAction() awaits _save(context) and then unconditionally calls setState. If the page is closed while the save dialog is running, this can still hit setState() after dispose.

Suggested fix
     pack!.images[imageCode] = newImageController.value!;
     await _save(context);
+    if (!mounted) return;
     setState(() {
       newImageCodeController.text = '';
       newImageController.value = null;
       showSave = false;
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_emotes/settings_emotes.dart` around
lines 179 - 216, The code calls setState after awaiting _save(context) in the
addImageAction flow without checking whether the widget is still mounted, which
can cause setState-after-dispose errors; update the logic in the block that
handles adding an image (references: newImageCodeController, newImageController,
pack!.images, _save, setState) to check mounted before calling setState (e.g.,
only perform the state reset/newImageController.value = null and showSave =
false inside an if (mounted) { setState(...) } after the await), and ensure any
early returns still leave data consistent.
lib/pages/settings_dashboard/settings_stories/settings_stories.dart (1)

24-50: ⚠️ Potential issue | 🟠 Major

Add mounted guards before these async setState calls.

Both branches update users after awaited network work inside the loading-dialog future. Leaving the page mid-request can still call setState() on a disposed controller.

Suggested fix
       await TwakeDialog.showFutureLoadingDialogFullScreen(
         future: () async {
           await user.kick();
           await room.client.setStoriesBlockList(blockList.toSet().toList());
+          if (!mounted) return;
           setState(() => users[user] = false);
         },
       );
@@
     await TwakeDialog.showFutureLoadingDialogFullScreen(
       future: () async {
         await room.client.setStoriesBlockList(blockList);
         await room.invite(user.id);
+        if (!mounted) return;
         setState(() => users[user] = true);
       },
     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_stories/settings_stories.dart` around
lines 24 - 50, In toggleUser, guard the async closures passed to
TwakeDialog.showFutureLoadingDialogFullScreen so they only call setState when
the State is still mounted: after awaiting network ops in both branches (the
future in the "kick" branch and the future in the "invite" branch) check if
(!mounted) return; before calling setState(() => users[user] = ...); and keep
all other logic unchanged; reference the toggleUser method and the futures
passed into TwakeDialog.showFutureLoadingDialogFullScreen to locate where to
insert the mounted checks.
lib/pages/settings_dashboard/settings/settings.dart (1)

78-103: ⚠️ Potential issue | 🟠 Major

Add mounted guards after awaited dialogs/routes.

These handlers resume into state-owned work after an await (matrix/context access in logoutAction() and checkBootstrap(), notifier writes after context.push(...)). If this page is disposed while the dialog or child route is open, the continuation can run against a dead State.

🛡️ Suggested guard
   await BootstrapDialog(client: Matrix.of(context).client).show();
+  if (!mounted) return;
   checkBootstrap();

Apply the same if (!mounted) return; immediately after the confirm dialog in logoutAction() and after each await context.push(...).

Also applies to: 201-213, 216-255

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings/settings.dart` around lines 78 - 103,
The continuation after awaited UI operations can run on a disposed State; add
mounted guards to prevent that by inserting if (!mounted) return; immediately
after the await showConfirmAlertDialog(...) in logoutAction(), and likewise
after each await context.push(...) (and any other await that resumes into
State-owned work such as notifier writes) in checkBootstrap() and other
handlers; ensure you check mounted before accessing context, matrix, or calling
_logoutActionsOnMobile(), _logoutActions(), or writing to notifiers so the
resumed code exits early if the State was disposed.
🧹 Nitpick comments (6)
lib/pages/settings_dashboard/settings_app_language/settings_app_language_view.dart (1)

73-73: Consider removing the force-unwrap on bodySmall for safer theming edge cases.

Line 73 uses textTheme.bodySmall!; a fallback avoids a potential runtime null assertion if a custom theme omits that style.

Optional safe fallback
-                        style: textTheme.bodySmall!.copyWith(
+                        style: (textTheme.bodySmall ?? const TextStyle()).copyWith(
                           color: LinagoraRefColors.material().neutral[40],
                         ),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@lib/pages/settings_dashboard/settings_app_language/settings_app_language_view.dart`
at line 73, The code force-unwraps textTheme.bodySmall (textTheme.bodySmall!)
which can crash if a custom theme omits that style; update the widget that
builds the label in SettingsAppLanguageView (the build method using
textTheme.bodySmall) to avoid the ! by providing a fallback TextStyle, e.g.
replace textTheme.bodySmall!.copyWith(...) with (textTheme.bodySmall ??
Theme.of(context).textTheme.bodyText2 ?? const TextStyle()).copyWith(...) so you
safely handle null bodySmall while preserving the intended copyWith
modifications.
lib/pages/settings_dashboard/settings_3pid/settings_3pid.dart (1)

62-62: Consider adding mounted check before setState.

Since the PR objectives mention adding mounted checks after async operations, you might want to add one here for consistency:

if (!mounted) return;
setState(() => request = null);

The same applies to line 85 in delete3Pid. This prevents potential setState calls on an unmounted widget after async operations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_3pid/settings_3pid.dart` at line 62,
Add a mounted check before calling setState to avoid updating an unmounted
widget: replace the plain setState(() => request = null); with a guard like if
(!mounted) return; setState(() => request = null); and apply the same pattern
inside the delete3Pid method before any setState calls there so that both the
request-clearing and delete3Pid state updates are conditional on the widget
still being mounted.
lib/pages/settings_dashboard/settings_emotes/settings_emotes_view.dart (1)

24-29: Minor inconsistency in theme caching.

Line 27 accesses Theme.of(context).colorScheme.secondary directly, while line 25 already caches Theme.of(context) into theme. Consider using theme.colorScheme.secondary for consistency.

Suggested fix
     final l10n = L10n.of(context)!;
     final theme = Theme.of(context);
     final textTheme = TextStyle(
-      color: Theme.of(context).colorScheme.secondary,
+      color: theme.colorScheme.secondary,
       fontWeight: FontWeight.bold,
     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_emotes/settings_emotes_view.dart`
around lines 24 - 29, The textStyle initialization inconsistently calls
Theme.of(context) instead of using the cached theme variable; update the
TextStyle where color is set to use theme.colorScheme.secondary (referencing the
existing theme local variable and the textTheme symbol) so both accesses use the
same cached Theme object.
lib/pages/settings_dashboard/settings_blocked_users/settings_blocked_users_view.dart (1)

142-158: Create a constant for the draftChat route in AppRoutePaths.

The hardcoded /rooms/draftChat route is used throughout the codebase (also appearing in search.dart, contacts_tab.dart, profile_info_body.dart, and chat_profile_info_details.dart) but lacks a corresponding constant in AppRoutePaths. Add a constant like static const String draftChatFull = '/rooms/draftChat'; to centralize this route definition, consistent with the PR objective.

The /rooms/$roomId navigation is a generic room navigation pattern and doesn't require a centralized constant.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@lib/pages/settings_dashboard/settings_blocked_users/settings_blocked_users_view.dart`
around lines 142 - 158, The code uses the hardcoded route string
'/rooms/draftChat' directly (e.g., the context.go call in
settings_blocked_users_view.dart), so add a centralized constant to
AppRoutePaths (e.g., static const String draftChatFull = '/rooms/draftChat';)
and replace all occurrences of the literal '/rooms/draftChat' across the
codebase (search.dart, contacts_tab.dart, profile_info_body.dart,
chat_profile_info_details.dart, and settings_blocked_users_view.dart) to
reference AppRoutePaths.draftChatFull to keep the route definition consistent.
lib/pages/settings_dashboard/settings_profile/settings_profile.dart (1)

487-494: Use the shared clipboard helper in this copy path too.

This touched branch still goes straight to Clipboard.setData(...) and shows the success snackbar immediately. Please route it through TwakeClipboard.instance.copyText(...) as well so the settings cleanup uses one clipboard path consistently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_profile/settings_profile.dart` around
lines 487 - 494, The .matrixId branch in copyEventsAction still calls
Clipboard.setData(...) and shows TwakeSnackBar.show directly; replace that
direct clipboard usage with TwakeClipboard.instance.copyText(...) (passing
client.mxid(context)) so the shared clipboard helper is used and its own success
handling is relied upon instead of showing the snackbar manually; update the
.matrixId case inside copyEventsAction and remove the direct
Clipboard.setData/TwakeSnackBar.show calls.
lib/pages/settings_dashboard/settings_profile/settings_profile_view_web.dart (1)

56-64: Keep the extracted helper widgets private.

PersonalInfosColumn, AvatarAndNameRow, and EditMenuBtn look file-local to SettingsProfileViewWeb. Leaving them public grows the importable API surface for a refactor-only change.

🔒 Suggested rename
-              AvatarAndNameRow(
+              _AvatarAndNameRow(
                 settingsProfileUIState: settingsProfileUIState,
                 onImageLoaded: onImageLoaded,
                 currentProfile: currentProfile,
                 client: client,
                 canEditAvatar: canEditAvatar,
                 menuController: menuController,
                 menuChildren: menuChildren,
                 basicInfoWidget: basicInfoWidget,
               ),
@@
-              PersonalInfosColumn(
+              _PersonalInfosColumn(
                 workIdentitiesInfoWidget: workIdentitiesInfoWidget,
               ),
@@
-class PersonalInfosColumn extends StatelessWidget {
-  const PersonalInfosColumn({
+class _PersonalInfosColumn extends StatelessWidget {
+  const _PersonalInfosColumn({
@@
-class AvatarAndNameRow extends StatelessWidget {
-  const AvatarAndNameRow({
+class _AvatarAndNameRow extends StatelessWidget {
+  const _AvatarAndNameRow({
@@
-                      EditMenuBtn(
+                      _EditMenuBtn(
                         menuController: menuController,
                         menuChildren: menuChildren,
                       ),
@@
-class EditMenuBtn extends StatelessWidget {
-  const EditMenuBtn({
+class _EditMenuBtn extends StatelessWidget {
+  const _EditMenuBtn({

Also applies to: 76-77, 87-145, 245-248, 261-266

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/pages/settings_dashboard/settings_profile/settings_profile_view_web.dart`
around lines 56 - 64, The helper widgets PersonalInfosColumn, AvatarAndNameRow,
and EditMenuBtn are currently public; make them file-private by renaming each
class to start with an underscore (e.g., _PersonalInfosColumn,
_AvatarAndNameRow, _EditMenuBtn) and update every instantiation and reference
within this file (for example where SettingsProfileViewWeb constructs
AvatarAndNameRow and PersonalInfosColumn) to use the new names; ensure
constructors, types, and any references (including menuChildren/basicInfoWidget
wiring) are updated and that no external imports rely on these now-private
widgets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@lib/pages/settings_dashboard/settings_profile/settings_profile_view_web.dart`:
- Around line 291-312: Replace the non-focusable GestureDetector used in builder
with a focusable IconButton so keyboard users can open/close the avatar menu:
keep the existing Container (and its BoxDecoration, padding, and Icon) as the
IconButton's child, and change the onTap lambda to an onPressed that toggles
MenuController.isOpen by calling menuController.close() or
menuController.open(); preserve SettingsProfileViewWebStyle constants
(avatarSize, iconEditBorderWidth, paddingEditIcon, iconEditSize) and consider
adding a tooltip for accessibility.

In `@lib/pages/settings_dashboard/settings_security/settings_security.dart`:
- Around line 169-171: In copyPublicKey(), await the asynchronous clipboard
write call so the snackbar is shown only after a successful write: change the
call to await TwakeClipboard.instance.copyText(client.fingerprintKey.beautified)
and then call TwakeSnackBar.show(context,
L10n.of(context)!.copiedPublicKeyToClipboard); optionally catch and handle
exceptions from copyText to show an error snackbar on failure.
- Around line 89-95: Move the AppLock lookup so it only runs after verifying the
widget is still mounted: after awaiting const FlutterSecureStorage().read(key:
SettingKeys.appLockKey) perform the mounted check (if (!mounted) return;) and
then call AppLock.of(context) to get the appLock; update any subsequent usage
that currently depends on the earlier appLock variable to use the newly
retrieved instance so you don't access context when the widget may have been
disposed.
- Around line 56-80: The call to client.changePassword is using a nonexistent
oldPassword parameter and lacks validation for matching passwords; update the
logic in the settings_security widget that builds the two DialogTextField inputs
and the call to TwakeDialog.showFutureLoadingDialogFullScreen so that you first
verify input.first == input.last and show an error if they differ, then call
client.changePassword with only the new password (input.last) and, if server UIA
is required, supply the proper auth object instead of an oldPassword argument
(use Matrix UIA flow). Ensure you reference the changePassword call and the
DialogTextField inputs when making the changes.

In `@lib/pages/settings_dashboard/settings_style/settings_style.dart`:
- Around line 34-38: The current setChatColor method short-circuits when color
is null, preventing the reset behavior; change it to always call
controller.setPrimaryColor(color) (allowing a null argument) and only update
AppConfig.colorSchemeSeed when color is non-null, so
ThemeController.setPrimaryColor(null) can clear the saved override while
preserving the stored seed via AppConfig.colorSchemeSeed.

---

Outside diff comments:
In `@lib/pages/settings_dashboard/settings_emotes/settings_emotes.dart`:
- Around line 179-216: The code calls setState after awaiting _save(context) in
the addImageAction flow without checking whether the widget is still mounted,
which can cause setState-after-dispose errors; update the logic in the block
that handles adding an image (references: newImageCodeController,
newImageController, pack!.images, _save, setState) to check mounted before
calling setState (e.g., only perform the state reset/newImageController.value =
null and showSave = false inside an if (mounted) { setState(...) } after the
await), and ensure any early returns still leave data consistent.

In `@lib/pages/settings_dashboard/settings_stories/settings_stories.dart`:
- Around line 24-50: In toggleUser, guard the async closures passed to
TwakeDialog.showFutureLoadingDialogFullScreen so they only call setState when
the State is still mounted: after awaiting network ops in both branches (the
future in the "kick" branch and the future in the "invite" branch) check if
(!mounted) return; before calling setState(() => users[user] = ...); and keep
all other logic unchanged; reference the toggleUser method and the futures
passed into TwakeDialog.showFutureLoadingDialogFullScreen to locate where to
insert the mounted checks.

In `@lib/pages/settings_dashboard/settings/settings.dart`:
- Around line 78-103: The continuation after awaited UI operations can run on a
disposed State; add mounted guards to prevent that by inserting if (!mounted)
return; immediately after the await showConfirmAlertDialog(...) in
logoutAction(), and likewise after each await context.push(...) (and any other
await that resumes into State-owned work such as notifier writes) in
checkBootstrap() and other handlers; ensure you check mounted before accessing
context, matrix, or calling _logoutActionsOnMobile(), _logoutActions(), or
writing to notifiers so the resumed code exits early if the State was disposed.

---

Nitpick comments:
In `@lib/pages/settings_dashboard/settings_3pid/settings_3pid.dart`:
- Line 62: Add a mounted check before calling setState to avoid updating an
unmounted widget: replace the plain setState(() => request = null); with a guard
like if (!mounted) return; setState(() => request = null); and apply the same
pattern inside the delete3Pid method before any setState calls there so that
both the request-clearing and delete3Pid state updates are conditional on the
widget still being mounted.

In
`@lib/pages/settings_dashboard/settings_app_language/settings_app_language_view.dart`:
- Line 73: The code force-unwraps textTheme.bodySmall (textTheme.bodySmall!)
which can crash if a custom theme omits that style; update the widget that
builds the label in SettingsAppLanguageView (the build method using
textTheme.bodySmall) to avoid the ! by providing a fallback TextStyle, e.g.
replace textTheme.bodySmall!.copyWith(...) with (textTheme.bodySmall ??
Theme.of(context).textTheme.bodyText2 ?? const TextStyle()).copyWith(...) so you
safely handle null bodySmall while preserving the intended copyWith
modifications.

In
`@lib/pages/settings_dashboard/settings_blocked_users/settings_blocked_users_view.dart`:
- Around line 142-158: The code uses the hardcoded route string
'/rooms/draftChat' directly (e.g., the context.go call in
settings_blocked_users_view.dart), so add a centralized constant to
AppRoutePaths (e.g., static const String draftChatFull = '/rooms/draftChat';)
and replace all occurrences of the literal '/rooms/draftChat' across the
codebase (search.dart, contacts_tab.dart, profile_info_body.dart,
chat_profile_info_details.dart, and settings_blocked_users_view.dart) to
reference AppRoutePaths.draftChatFull to keep the route definition consistent.

In `@lib/pages/settings_dashboard/settings_emotes/settings_emotes_view.dart`:
- Around line 24-29: The textStyle initialization inconsistently calls
Theme.of(context) instead of using the cached theme variable; update the
TextStyle where color is set to use theme.colorScheme.secondary (referencing the
existing theme local variable and the textTheme symbol) so both accesses use the
same cached Theme object.

In
`@lib/pages/settings_dashboard/settings_profile/settings_profile_view_web.dart`:
- Around line 56-64: The helper widgets PersonalInfosColumn, AvatarAndNameRow,
and EditMenuBtn are currently public; make them file-private by renaming each
class to start with an underscore (e.g., _PersonalInfosColumn,
_AvatarAndNameRow, _EditMenuBtn) and update every instantiation and reference
within this file (for example where SettingsProfileViewWeb constructs
AvatarAndNameRow and PersonalInfosColumn) to use the new names; ensure
constructors, types, and any references (including menuChildren/basicInfoWidget
wiring) are updated and that no external imports rely on these now-private
widgets.

In `@lib/pages/settings_dashboard/settings_profile/settings_profile.dart`:
- Around line 487-494: The .matrixId branch in copyEventsAction still calls
Clipboard.setData(...) and shows TwakeSnackBar.show directly; replace that
direct clipboard usage with TwakeClipboard.instance.copyText(...) (passing
client.mxid(context)) so the shared clipboard helper is used and its own success
handling is relied upon instead of showing the snackbar manually; update the
.matrixId case inside copyEventsAction and remove the direct
Clipboard.setData/TwakeSnackBar.show calls.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d2e0662-44f1-45cb-b59e-9ee1ef477360

📥 Commits

Reviewing files that changed from the base of the PR and between bc4b54a and a6fbc99.

📒 Files selected for processing (41)
  • lib/config/go_routes/app_route_paths.dart
  • lib/config/go_routes/go_router.dart
  • lib/pages/chat_list/chat_list.dart
  • lib/pages/contacts_tab/contacts_tab.dart
  • lib/pages/multiple_accounts/multiple_accounts_picker.dart
  • lib/pages/search/search.dart
  • lib/pages/settings_dashboard/settings/settings.dart
  • lib/pages/settings_dashboard/settings/settings_item_builder.dart
  • lib/pages/settings_dashboard/settings_3pid/settings_3pid.dart
  • lib/pages/settings_dashboard/settings_3pid/settings_3pid_view.dart
  • lib/pages/settings_dashboard/settings_app_language/settings_app_language_view.dart
  • lib/pages/settings_dashboard/settings_blocked_users/settings_blocked_user.dart
  • lib/pages/settings_dashboard/settings_blocked_users/settings_blocked_users_view.dart
  • lib/pages/settings_dashboard/settings_chat/settings_chat_view.dart
  • lib/pages/settings_dashboard/settings_contacts_visibility/settings_contacts_visibility.dart
  • lib/pages/settings_dashboard/settings_contacts_visibility/settings_contacts_visibility_enum.dart
  • lib/pages/settings_dashboard/settings_contacts_visibility/settings_contacts_visibility_view.dart
  • lib/pages/settings_dashboard/settings_emotes/settings_emotes.dart
  • lib/pages/settings_dashboard/settings_emotes/settings_emotes_view.dart
  • lib/pages/settings_dashboard/settings_multiple_emotes/settings_multiple_emotes_view.dart
  • lib/pages/settings_dashboard/settings_notifications/settings_notifications_view.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_context_menu_actions.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_item.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_item_style.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_redirection_edit_button.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_state/settings_profile_ui_state.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view_mobile.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view_mobile_style.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view_style.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view_web.dart
  • lib/pages/settings_dashboard/settings_profile/settings_profile_view_web_style.dart
  • lib/pages/settings_dashboard/settings_security/settings_security.dart
  • lib/pages/settings_dashboard/settings_security/settings_security_view.dart
  • lib/pages/settings_dashboard/settings_stories/settings_stories.dart
  • lib/pages/settings_dashboard/settings_stories/settings_stories_view.dart
  • lib/pages/settings_dashboard/settings_style/settings_style.dart
  • lib/pages/settings_dashboard/settings_style/settings_style_view.dart
  • lib/pages/twake_welcome/twake_welcome.dart
  • lib/widgets/layouts/adaptive_layout/adaptive_scaffold_primary_navigation_view.dart
💤 Files with no reviewable changes (1)
  • lib/pages/settings_dashboard/settings_profile/settings_profile_state/settings_profile_ui_state.dart

@9clg6 9clg6 force-pushed the refacto/TW-2991-settings-router-code-hygiene branch from a6fbc99 to 5e19099 Compare March 19, 2026 13:49
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
lib/config/go_routes/app_route_paths.dart (1)

11-62: Consider extracting a shared roomsRoot constant to avoid path drift.

'/rooms/' is repeated across many *Full constants. Centralizing the base path will make future path migrations safer.

♻️ Suggested refactor
 abstract class AppRoutePaths {
+  static const String roomsRoot = '/rooms';
+
   // Settings prefix check
-  static const String roomsSettings = '/rooms/settings';
+  static const String settingsSegment = 'settings';
+  static const String roomsSettings = '$roomsRoot/$settingsSegment';

   // Profile routes
   static const String profileSegment = 'profile';
-  static const String profileFull = '/rooms/$profileSegment';
+  static const String profileFull = '$roomsRoot/$profileSegment';
   static const String profileQrSegment = 'qr';
   static const String profileQrFull = '$profileFull/$profileQrSegment';

   // Chat settings routes
   static const String chatSegment = 'chat';
-  static const String chatFull = '/rooms/$chatSegment';
+  static const String chatFull = '$roomsRoot/$chatSegment';

   // Security routes
   static const String securitySegment = 'security';
-  static const String roomsSecurityFull = '/rooms/$securitySegment';
+  static const String roomsSecurityFull = '$roomsRoot/$securitySegment';

   // Notifications routes
   static const String notificationsSegment = 'notifications';
-  static const String notificationsFull = '/rooms/$notificationsSegment';
+  static const String notificationsFull = '$roomsRoot/$notificationsSegment';

   // Style routes
   static const String styleSegment = 'style';
-  static const String styleFull = '/rooms/$styleSegment';
+  static const String styleFull = '$roomsRoot/$styleSegment';

   // App language routes
   static const String appLanguageSegment = 'appLanguage';
-  static const String appLanguageFull = '/rooms/$appLanguageSegment';
+  static const String appLanguageFull = '$roomsRoot/$appLanguageSegment';

   // Devices routes
   static const String devicesSegment = 'devices';
-  static const String devicesFull = '/rooms/$devicesSegment';
+  static const String devicesFull = '$roomsRoot/$devicesSegment';

   // Add account routes
   static const String addAccountSegment = 'addaccount';
-  static const String addAccountFull = '/rooms/$addAccountSegment';
+  static const String addAccountFull = '$roomsRoot/$addAccountSegment';
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/config/go_routes/app_route_paths.dart` around lines 11 - 62, Many Full
path constants repeat the '/rooms' prefix; introduce a single base constant
(e.g. static const String roomsRoot = '/rooms';) and replace all occurrences
that build paths with '/rooms/...' (such as profileFull, chatFull,
roomsSecurityFull, notificationsFull, styleFull, appLanguageFull, devicesFull,
addAccountFull) to derive from roomsRoot (e.g. '$roomsRoot/$profileSegment' or
string concat appropriate to your chosen separator), ensuring consistent
trailing-slash handling across profileFull, chatFull, roomsSecurityFull,
notificationsFull, styleFull, appLanguageFull, devicesFull and addAccountFull so
future path changes require updating only roomsRoot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/config/go_routes/app_route_paths.dart`:
- Around line 11-62: Many Full path constants repeat the '/rooms' prefix;
introduce a single base constant (e.g. static const String roomsRoot =
'/rooms';) and replace all occurrences that build paths with '/rooms/...' (such
as profileFull, chatFull, roomsSecurityFull, notificationsFull, styleFull,
appLanguageFull, devicesFull, addAccountFull) to derive from roomsRoot (e.g.
'$roomsRoot/$profileSegment' or string concat appropriate to your chosen
separator), ensuring consistent trailing-slash handling across profileFull,
chatFull, roomsSecurityFull, notificationsFull, styleFull, appLanguageFull,
devicesFull and addAccountFull so future path changes require updating only
roomsRoot.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c64b3f5f-4e72-4752-8b87-0519fe4ccb10

📥 Commits

Reviewing files that changed from the base of the PR and between 5e19099 and c221065.

📒 Files selected for processing (2)
  • lib/config/go_routes/app_route_paths.dart
  • lib/config/go_routes/go_router.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/config/go_routes/go_router.dart

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.

1 participant