-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix/misc/profile settings extract strings #337
Fix/misc/profile settings extract strings #337
Conversation
- Add string resources for CreateProfileScreen (screen title, radius explanation text) to `strings.xml`. - Refactor CreateProfileScreen to use the newly added string resources. - Update tests to verify usage of the correct string resources.
- Add string resource for EditProfileScreen (screen title) to `strings.xml`. - Refactor EditProfileScreen to use the newly added string resource. - Update tests to verify usage of the correct string resource.
- Add string resources for ProfileScreen (screen title, default name, default description, user status, number of interactions, reviews title, and no reviews message) to `strings.xml`. - Refactor ProfileScreen to use the newly added string resources. - Update tests to verify usage of the correct string resources.
- Add string resources for SettingsScreen (screen title, comments, notifications, themes, account management, dialog text, log messages, and toast messages) to `strings.xml`. - Refactor SettingsScreen to use the newly added string resources. - Update tests to verify usage of the correct string resources.
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.
Summary
Very nice refactoring of our string constants, thank you for the work!
Important
Code Quality
This improves consistency and readability, very nice!
Functionality
Works as before, all good here!
Testing
Good adaptation of the tests to fetch from the xml.
Documentation
No notes!
Suggestions
See comments. You also forgot to replace some strings in the Profile screen tests at around line 230 (GitHub wouldn't let me directly post a comment for some reason, sorry).
Steps before PR approved
See comments, as well as the forgotten strings mentioned. Otherwise, LGTM
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.
LGTM ! Just do not forget to read my comment review !
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
…-strings # Conflicts: # app/src/main/res/values/strings.xml
Quality Gate passedIssues Measures |
Create Profile, Edit Profile, Profile, and Settings texts in
strings.xml
Description
This PR introduces string resources for the Create Profile, Edit Profile, Profile, and Settings screens to improve readability and maintainability across the app. It ensures the content of these screens is easily managed through the
strings.xml
file. This update closes issue #336 (and part of #328).Changes
CreateProfileScreen
,EditProfileScreen
,ProfileScreen
,SettingsScreen
) to use the newly added string resources, ensuring consistency and ease of maintenance.Files
Added
Modified
CreateProfileScreen.kt
(updated to use string resources fromstrings.xml
)EditProfileScreen.kt
(updated to use string resources fromstrings.xml
)ProfileScreen.kt
(updated to use string resources fromstrings.xml
)SettingsScreen.kt
(updated to use string resources fromstrings.xml
)res/values/strings.xml
(new strings added for Create Profile, Edit Profile, Profile, and Settings)Removed
Dependencies Added
Testing
The Create Profile, Edit Profile, Profile, and Settings screens have been updated to ensure the correct string resources are being used. Relevant tests have been adjusted to verify proper usage of the new resources.