-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
base: main
Are you sure you want to change the base?
Conversation
"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." |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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 |
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 |
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 |
There was a problem hiding this 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 inlib/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 3Length of output: 13527
🧹 Nitpick comments (14)
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart (3)
1-1
: Update copyright yearThe 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 messagesThe 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 usersThe 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
⛔ 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:
- Adding the required localization import
- Using
context.l10n
to retrieve the localized string for the app bar titleAlso 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:
- Adding the required localization import
- Using
context.l10n
to retrieve the localized string for the team section titleAlso 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 redundantstateEnumToString
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 20Length 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 issueFix 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.
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM
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: