Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate most of the setting pages #1802

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

nilsreichardt
Copy link
Member

@nilsreichardt nilsreichardt commented Jan 10, 2025

First of all, sorry for the big PR. I translated all the strings when I was on the plane. Splitting this PR into multiple PRs causes a lot of merge conflicts. So I would just do one big PR.

This PR adds translations for the following pages:

  • Settings: My profile page
  • Settings: Change Name page
  • Settings: Change email page
  • Settings: Change password page
  • Settings: Change status page
  • Settings: About page
  • Settings: Theme page
  • Settings: Timetable settings page (may be incomplete)

@github-actions github-actions bot added legal Regarding Licenses, Policy updates, Warnings to users (that might cause trouble if not there) etc. feature: timetable / calendar Includes anything regarding lessons (timetable) and events (calendar). ui / ux feature: groups Groups umbrella term for courses and classes. feature: navigation Navigation inside the app (e.g. switching to a different screen). dependencies Changing, updating, adding or removing one or more dependencies. labels Jan 10, 2025
"changeTypeOfUserPagePermissionNote": "Beachte die folgende Hinweise:\n* Innerhalb von 14 Tagen kannst du nur 2x den Account-Typ ändern.\n* Durch das Ändern der Nutzer erhältst du keine weiteren Berechtigungen in den Gruppen. Ausschlaggebend sind die Gruppenberechtigungen (\"Administrator\", \"Aktives Mitglied\", \"Passives Mitglied\").",
"changeTypeOfUserPageRestartAppDialogTitle": "Neustart erforderlich",
"@changeTypeOfUserPageRestartAppDialogTitle": {
"description": "The title of the dialog which will be displayed after a successful type of user change to explain that a restart is required."
Copy link
Member Author

Choose a reason for hiding this comment

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

I have to check if it's better to write the description tags in German (it's the de.arb file) or in English. I assume they are used to add more context to the LLM.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's better to use English. Currently, I'm using German as default language because our hard-coded strings are in German and we can easily auto-generate the English ones. However, once we translated the entire app, we should change the default language to English.

Copy link

github-actions bot commented Jan 10, 2025

Visit the preview URL for this PR (updated for commit 11edeaf):

https://sharezone-console-dev--pr1802-translate-setttings-rjvab3h4.web.app

(expires Mon, 13 Jan 2025 09:50:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 471536afe3f6ec4895d9ea75513730b515d17eb6

Copy link

github-actions bot commented Jan 10, 2025

Visit the preview URL for this PR (updated for commit 11edeaf):

https://sharezone-website-dev--pr1802-translate-setttings-0wy7vxz5.web.app

(expires Mon, 13 Jan 2025 09:50:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 372b0431a96247f908d9a97d5d865de1c8b3b04e

Copy link

Visit the preview URL for this PR (updated for commit df62898):

https://sharezone-test--pr1802-translate-setttings-1ibonwf4.web.app

(expires Mon, 13 Jan 2025 09:15:55 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4cb3ae61e1e018abfd9841fd3239f5b49ccc034b

@nilsreichardt nilsreichardt marked this pull request as draft January 10, 2025 09:17
Copy link
Contributor

@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: 12

🔭 Outside diff range comments (2)
lib/sharezone_localizations/l10n/app_en.arb (1)

Line range hint 221-222: Fix duplicate text for lesson length title and subtitle.

The title and subtitle for the lesson length settings have the same text, which seems incorrect. The subtitle should provide additional context or explanation.

Apply this diff to make the subtitle more descriptive:

    "timetableSettingsPageLessonLengthTileTile": "Länge einer Stunde",
-   "timetableSettingsPageLessonLengthTileSubtile": "Länge einer Stunde",
+   "timetableSettingsPageLessonLengthTileSubtile": "Dauer einer Unterrichtsstunde in Minuten",
app/lib/settings/src/subpages/my_profile/change_type_of_user/change_type_of_user_page.dart (1)

Hardcoded German strings need localization

The toReadableString() method in lib/user/lib/src/models/type_of_user.dart uses hardcoded German strings (e.g., 'Schüler*in') without any localization mechanism. Since these strings are used across multiple features (settings, onboarding, groups, etc.), they should be properly localized to support multiple languages.

🔗 Analysis chain

Line range hint 195-205: Consider localizing type of user strings.

The toReadableString() method is used to display user types, but these strings might not be localized. Consider using localized strings for user types as well.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if toReadableString() uses localized strings
ast-grep --pattern 'toReadableString() {
  $$$
}'

Length of output: 51


Script:

#!/bin/bash
# Find TypeOfUser enum definition and its toReadableString method
rg "enum TypeOfUser" -A 10
rg "toReadableString" -A 3 -B 3

Length of output: 13527

🧹 Nitpick comments (14)
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart (3)

1-1: Update copyright year

The copyright year is set to 2025, but we're currently in January 2025. Consider updating it to 2024 or using a range (e.g., 2024-2025) to reflect when the code was actually written.

-// Copyright (c) 2025 Sharezone UG (haftungsbeschränkt)
+// Copyright (c) 2024-2025 Sharezone UG (haftungsbeschränkt)

217-220: Consider a more formal tone for security-related messages

The automatic sign-out/sign-in message uses an emoji and casual language ("also nicht wundern 😉"). For security-related operations, consider using a more formal tone to emphasize the importance of the action.

-      'Hinweis: Wenn deine E-Mail geändert wurde, wirst du automatisch kurz ab- und sofort wieder angemeldet - also nicht wundern 😉';
+      'Hinweis: Nach der Änderung deiner E-Mail-Adresse erfolgt aus Sicherheitsgründen eine automatische Ab- und Wiederanmeldung.';

417-420: Improve clarity of feedback channels for non-technical users

The feedback instructions mention "Feedback-Box" and "Discord-Server" without providing context or alternatives for users who might not be familiar with these channels.

-      'Wir testen aktuell eine neue Navigation. Bitte gib über die Feedback-Box oder unseren Discord-Server eine kurze Rückmeldung, wie du die jeweiligen Optionen findest.';
+      'Wir testen aktuell eine neue Navigation. Bitte teile uns deine Meinung mit - entweder über das Feedback-Formular in der App oder in unserer Online-Community (Discord). Du findest beide Optionen im Hilfe-Bereich.';
app/lib/navigation/scaffold/portable/bottom_navigation_bar/navigation_experiment/navigation_experiment_option.dart (1)

9-10: LGTM! Consider adding documentation.

The implementation is well-structured using a switch expression and properly implements localization. Consider adding a documentation comment for the getDisplayName method to explain its purpose and usage.

 extension ToReadableString on NavigationExperimentOption {
+  /// Returns the localized display name for this navigation experiment option.
   String getDisplayName(BuildContext context) {

Also applies to: 25-33

app/lib/settings/src/subpages/about/widgets/member_card.dart (1)

51-52: LGTM! Consider improving formatting for readability.

The changes correctly pass the context to all social buttons. Consider improving the formatting for better readability.

-                    SocialButton.instagram(
-                        context, socialMediaLinks!.instagram!),
+                    SocialButton.instagram(context, socialMediaLinks!.instagram!),

Also applies to: 54-54, 56-58

app/lib/settings/src/subpages/my_profile/change_state.dart (1)

46-48: Consider adding a retry option for error state.

When the state fails to load, we only show an error message. Consider adding a retry button to improve user experience.

 if (snapshot.hasError) {
-  return Text(context.l10n.changeStatePageErrorLoadingState);
+  return Center(
+    child: Column(
+      mainAxisAlignment: MainAxisAlignment.center,
+      children: [
+        Text(context.l10n.changeStatePageErrorLoadingState),
+        const SizedBox(height: 16),
+        ElevatedButton(
+          onPressed: () => bloc.reload(),
+          child: Text(context.l10n.retry),
+        ),
+      ],
+    ),
+  );
 }
app/lib/settings/src/subpages/my_profile/change_email.dart (2)

87-89: Consider using theme-based text style.

Instead of hardcoding the text style, consider using the theme's caption style or creating a reusable style constant.

-              Text(
-                context.l10n.changeEmailAddressPageNoteOnAutomaticSignOutSignIn,
-                style: const TextStyle(color: Colors.grey, fontSize: 11),
-              ),
+              Text(
+                context.l10n.changeEmailAddressPageNoteOnAutomaticSignOutSignIn,
+                style: Theme.of(context).textTheme.bodySmall?.copyWith(
+                  color: Theme.of(context).hintColor,
+                ),
+              ),

123-125: Consider using theme-based styling for consistency.

The disabled TextField could benefit from using the theme's disabled text style.

-      style: const TextStyle(color: Colors.grey, fontSize: 16),
+      style: Theme.of(context).textTheme.bodyLarge?.copyWith(
+        color: Theme.of(context).disabledColor,
+      ),
app/lib/settings/src/subpages/about/about_page.dart (2)

73-83: Consider using theme-based text styles.

The header text styles should leverage the theme for better maintainability and consistency.

           style: TextStyle(
-            color:
-                Theme.of(context).isDarkTheme ? Colors.white : Colors.black54,
-            fontSize: 20,
-            fontWeight: FontWeight.bold,
+            color: Theme.of(context).textTheme.titleLarge?.color,
+            fontSize: Theme.of(context).textTheme.titleLarge?.fontSize,
+            fontWeight: Theme.of(context).textTheme.titleLarge?.fontWeight,
           ),
         ),
         Text(
           context.l10n.aboutPageHeaderSubtitle,
-          style: const TextStyle(
-            color: Colors.grey,
-            fontSize: 15,
-          ),
+          style: Theme.of(context).textTheme.bodyMedium?.copyWith(
+            color: Theme.of(context).hintColor,
+          ),

Line range hint 97-104: Use theme-based styling for version text.

The version number display should use theme-based styling for consistency.

              style: TextStyle(
-                  color: Colors.grey[400],
-                  fontSize: 14,
-                  fontStyle: FontStyle.italic),
+                  color: Theme.of(context).textTheme.bodySmall?.color,
+                  fontSize: Theme.of(context).textTheme.bodySmall?.fontSize,
+                  fontStyle: FontStyle.italic),
app/lib/settings/src/subpages/my_profile/change_password.dart (3)

41-42: Consider using string interpolation for better readability.

The multi-line string concatenation can be simplified.

-                labelText: context
-                    .l10n.changePasswordPageCurrentPasswordTextfieldLabel,
+                labelText: context.l10n.changePasswordPageCurrentPasswordTextfieldLabel,

133-136: Consider using string interpolation for better readability.

Similar to the previous comment, the multi-line string concatenation can be simplified.

-                  title: Text(context
-                      .l10n.changePasswordPageResetCurrentPasswordDialogTitle),
-                  content: Text(context.l10n
-                      .changePasswordPageResetCurrentPasswordDialogContent),
+                  title: Text(context.l10n.changePasswordPageResetCurrentPasswordDialogTitle),
+                  content: Text(context.l10n.changePasswordPageResetCurrentPasswordDialogContent),

169-170: Consider using string interpolation for better readability.

Similar string concatenation issue.

-              message = context.l10n
-                  .changePasswordPageResetCurrentPasswordEmailSentConfirmation;
+              message = context.l10n.changePasswordPageResetCurrentPasswordEmailSentConfirmation;
app/lib/settings/src/subpages/my_profile/change_type_of_user/change_type_of_user_page.dart (1)

102-112: Consider extracting error messages to a separate method.

The switch expression for error messages is becoming complex. Consider extracting it to a separate method for better maintainability.

+  String _getErrorMessage(BuildContext context, ChangeTypeOfUserFailed failure) {
+    return switch (failure) {
+      ChangeTypeOfUserUnknownException(error: final error) =>
+        context.l10n.changeTypeOfUserPageErrorDialogContentUnknown(error),
+      NoTypeOfUserSelectedException() =>
+        context.l10n.changeTypeOfUserPageErrorDialogContentNoTypeOfUserSelected,
+      TypeUserOfUserHasNotChangedException() =>
+        context.l10n.changeTypeOfUserPageErrorDialogContentTypeOfUserHasNotChanged,
+      ChangedTypeOfUserTooOftenException(blockedUntil: final blockedUntil) =>
+        context.l10n.changeTypeOfUserPageErrorDialogContentChangedTypeOfUserTooOften(
+          blockedUntil),
+    };
+  }

   @override
   Widget build(BuildContext context) {
     return MaxWidthConstraintBox(
       maxWidth: 500,
       child: AlertDialog(
         title: Text(context.l10n.changeTypeOfUserPageErrorDialogTitle),
-        content: Text(
-          switch (failure) {
-            ChangeTypeOfUserUnknownException(error: final error) =>
-              context.l10n.changeTypeOfUserPageErrorDialogContentUnknown(error),
-            NoTypeOfUserSelectedException() => context.l10n
-                .changeTypeOfUserPageErrorDialogContentNoTypeOfUserSelected,
-            TypeUserOfUserHasNotChangedException() => context.l10n
-                .changeTypeOfUserPageErrorDialogContentTypeOfUserHasNotChanged,
-            ChangedTypeOfUserTooOftenException(
-              blockedUntil: final blockedUntil
-            ) =>
-              context.l10n
-                  .changeTypeOfUserPageErrorDialogContentChangedTypeOfUserTooOften(
-                      blockedUntil),
-          },
-        ),
+        content: Text(_getErrorMessage(context, failure)),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a03a8a and df62898.

⛔ Files ignored due to path filters (6)
  • lib/authentification/authentification_base/pubspec.lock is excluded by !**/*.lock, !**/*.lock
  • lib/authentification/authentification_qrcode/pubspec.lock is excluded by !**/*.lock, !**/*.lock
  • lib/group_domain_implementation/pubspec.lock is excluded by !**/*.lock, !**/*.lock
  • lib/group_domain_models/pubspec.lock is excluded by !**/*.lock, !**/*.lock
  • lib/user/pubspec.lock is excluded by !**/*.lock, !**/*.lock
  • lib/user/pubspec.yaml is excluded by !**/*.yaml
📒 Files selected for processing (20)
  • app/lib/navigation/scaffold/portable/bottom_navigation_bar/navigation_experiment/navigation_experiment_option.dart (2 hunks)
  • app/lib/settings/src/subpages/about/about_page.dart (5 hunks)
  • app/lib/settings/src/subpages/about/widgets/member_card.dart (1 hunks)
  • app/lib/settings/src/subpages/about/widgets/social_media_button.dart (3 hunks)
  • app/lib/settings/src/subpages/about/widgets/team.dart (2 hunks)
  • app/lib/settings/src/subpages/imprint/page/imprint_page.dart (2 hunks)
  • app/lib/settings/src/subpages/my_profile/change_data.dart (1 hunks)
  • app/lib/settings/src/subpages/my_profile/change_email.dart (7 hunks)
  • app/lib/settings/src/subpages/my_profile/change_password.dart (7 hunks)
  • app/lib/settings/src/subpages/my_profile/change_state.dart (5 hunks)
  • app/lib/settings/src/subpages/my_profile/change_type_of_user/change_type_of_user_page.dart (6 hunks)
  • app/lib/settings/src/subpages/my_profile/my_profile_page.dart (20 hunks)
  • app/lib/settings/src/subpages/theme/theme_page.dart (10 hunks)
  • app/lib/settings/src/subpages/timetable/timetable_settings_page.dart (13 hunks)
  • lib/sharezone_localizations/l10n/app_de.arb (1 hunks)
  • lib/sharezone_localizations/l10n/app_en.arb (1 hunks)
  • lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (3 hunks)
  • lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart (3 hunks)
  • lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart (3 hunks)
  • lib/user/lib/src/models/state_enum.dart (2 hunks)
🧰 Additional context used
🪛 rubocop (1.69.1)
lib/sharezone_localizations/l10n/app_de.arb

[warning] 128-128: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: Deploy Preview
  • GitHub Check: web-preview
  • GitHub Check: analyze
  • GitHub Check: analyze
  • GitHub Check: test-with-goldens
  • GitHub Check: test-without-goldens
  • GitHub Check: analyze
🔇 Additional comments (25)
app/lib/settings/src/subpages/imprint/page/imprint_page.dart (1)

13-13: LGTM! Proper localization implementation.

The changes correctly implement localization by:

  1. Adding the required localization import
  2. Using context.l10n to retrieve the localized string for the app bar title

Also applies to: 24-24

app/lib/settings/src/subpages/about/widgets/team.dart (1)

11-11: LGTM! Proper localization implementation.

The changes correctly implement localization by:

  1. Adding the required localization import
  2. Using context.l10n to retrieve the localized string for the team section title

Also applies to: 31-31

lib/user/lib/src/models/state_enum.dart (2)

34-57: LGTM! Clean implementation of the localized display names.

The switch expression provides a clean and maintainable way to map enum values to localized strings. All cases are properly handled.


Line range hint 59-79: Consider removing the redundant stateEnumToString map.

Now that we have the getDisplayName method that provides localized strings, this map with hardcoded German strings appears to be redundant. If this map is not used elsewhere in the codebase, consider removing it.

Let's verify if this map is still being used:

app/lib/settings/src/subpages/about/widgets/social_media_button.dart (1)

Line range hint 71-79: LGTM! Proper error handling with localization.

The email error handling is well implemented:

  • Checks if context is mounted before showing the snackbar
  • Uses localized message for the confirmation
app/lib/settings/src/subpages/my_profile/change_data.dart (1)

73-75: LGTM! Good improvement to enforce localization.

Making labelText required instead of having a default hardcoded string ensures that all instances of this widget will use localized text.

app/lib/settings/src/subpages/my_profile/change_state.dart (1)

24-29: LGTM! Comprehensive localization of all user-facing strings.

All hardcoded strings have been properly replaced with localized versions using context.l10n.

Also applies to: 47-47, 72-72, 79-84, 133-133

app/lib/settings/src/subpages/my_profile/change_email.dart (3)

36-39: LGTM! AppBar title properly localized.

The AppBar title has been correctly updated to use localization.


53-53: LGTM! Good use of common action string.

Using commonActionsSave for the tooltip promotes consistency across the app.


106-108: LGTM! InfoMessage properly localized.

The InfoMessage component's title and message are correctly using localization.

app/lib/settings/src/subpages/about/about_page.dart (1)

130-134: Verify social media button tooltips localization.

Ensure that the SocialButton class properly uses the passed context for localizing tooltips.

✅ Verification successful

Social media button tooltips are properly localized

The SocialButton class correctly uses the BuildContext for tooltip localization through context.l10n in all social media button constructors.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if SocialButton class uses localization for tooltips
ast-grep --pattern 'class SocialButton {
  $$$
  tooltip($$$) {
    $$$
  }
  $$$
}'

Length of output: 86


Script:

#!/bin/bash
# Find SocialButton class definition
ast-grep --pattern 'class SocialButton'

# Backup search using ripgrep for more context
rg "class SocialButton" -A 20

Length of output: 2268

app/lib/settings/src/subpages/theme/theme_page.dart (8)

12-12: LGTM! Import changes look good.

The imports have been properly organized, adding the required localization package.

Also applies to: 19-20


32-35: LGTM! AppBar title has been properly localized.

The title now uses the localization framework correctly.


57-59: LGTM! Section title has been properly localized.

The title now uses the localization framework correctly.


79-93: LGTM! Radio button titles have been properly localized.

All radio button titles now use the localization framework correctly.


139-143: LGTM! Card title and content have been properly localized.

The title and content now use the localization framework correctly.


173-183: LGTM! Navigation experiment section has been properly localized.

The title and content now use the localization framework correctly.


286-290: LGTM! Navigation option tile has been properly localized.

The title now uses the localization framework correctly with proper parameter passing.


331-331: LGTM! Support button text has been properly localized.

The button text now uses the localization framework correctly.

lib/sharezone_localizations/l10n/app_en.arb (1)

128-130: ⚠️ Potential issue

Fix duplicate key in localization file.

The key "changeEmailAddressPageNewEmailTextfieldLabel" is duplicated. The second occurrence should be "changeEmailAddressPagePasswordTextfieldLabel" based on the label text.

Apply this diff to fix the duplicate key:

-   "@changeEmailAddressPageNewEmailTextfieldLabel": {
+   "@changeEmailAddressPagePasswordTextfieldLabel": {
        "description": "The label for the text field which is used for the password"
    },

Likely invalid or redundant comment.

app/lib/settings/src/subpages/timetable/timetable_settings_page.dart (2)

28-28: LGTM! Import and AppBar title changes look good.

The localization package has been properly imported and the AppBar title now uses the localization framework correctly.

Also applies to: 43-46


93-93: LGTM! Widget titles and content have been properly localized.

All widget titles and content now use the localization framework correctly with proper parameter passing where needed.

Also applies to: 108-109, 163-164, 180-182, 197-198, 221-221, 264-266, 327-327, 330-330, 336-336, 373-374

app/lib/settings/src/subpages/my_profile/my_profile_page.dart (1)

Line range hint 56-622: LGTM! Good job on the localization changes.

The changes correctly replace hardcoded strings with localized strings using context.l10n. This improves the internationalization of the app while maintaining the same functionality.

lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (1)

Line range hint 1-938: LGTM! Well-structured localization interface.

The changes properly define the interface for all localized strings with good documentation. The localization delegate is correctly implemented.

lib/sharezone_localizations/l10n/app_de.arb (1)

93-101: LGTM! Well-documented localization entries.

The descriptions for dialog messages provide good context for translators and maintainers.

@SharezoneApp SharezoneApp deleted a comment from coderabbitai bot Jan 10, 2025
Copy link
Collaborator

@Jonas-Sander Jonas-Sander left a comment

Choose a reason for hiding this comment

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

Nice, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changing, updating, adding or removing one or more dependencies. feature: groups Groups umbrella term for courses and classes. feature: navigation Navigation inside the app (e.g. switching to a different screen). feature: timetable / calendar Includes anything regarding lessons (timetable) and events (calendar). legal Regarding Licenses, Policy updates, Warnings to users (that might cause trouble if not there) etc. ui / ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants