Move TopicScreen strings to resources#2116
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded strings in the TopicScreen with localized string resources. The feedback recommends using sentence case for the string definitions and applying uppercase transformations within the UI layer to better support localization and separate content from presentation.
| <string name="feature_topic_api_following">FOLLOWING</string> | ||
| <string name="feature_topic_api_not_following">NOT FOLLOWING</string> |
There was a problem hiding this comment.
Avoid hardcoding uppercase text in string resources. Casing is a presentation detail that should be handled by the UI layer. This ensures better support for localization, as capitalization rules vary by language and some scripts do not use it at all. Using sentence case here also makes these strings consistent with the other new strings in this file.
| <string name="feature_topic_api_following">FOLLOWING</string> | |
| <string name="feature_topic_api_not_following">NOT FOLLOWING</string> | |
| <string name="feature_topic_api_following">Following</string> | |
| <string name="feature_topic_api_not_following">Not following</string> |
There was a problem hiding this comment.
Thanks for the suggestion. I updated the follow-state strings to sentence case in the resources.
| Text(stringResource(TopicR.string.feature_topic_api_following)) | ||
| } else { | ||
| Text("NOT FOLLOWING") | ||
| Text(stringResource(TopicR.string.feature_topic_api_not_following)) |
There was a problem hiding this comment.
To maintain the current all-caps design while using sentence-case string resources, apply the .uppercase() transformation in the UI. This separates the content from its presentation and follows Android best practices for internationalization.
| Text(stringResource(TopicR.string.feature_topic_api_following)) | |
| } else { | |
| Text("NOT FOLLOWING") | |
| Text(stringResource(TopicR.string.feature_topic_api_not_following)) | |
| Text(stringResource(TopicR.string.feature_topic_api_following).uppercase()) | |
| } else { | |
| Text(stringResource(TopicR.string.feature_topic_api_not_following).uppercase()) |
There was a problem hiding this comment.
Thanks, this makes sense. I kept the visible all-caps presentation in the UI and used uppercase(Locale.getDefault()) to match the existing production pattern in NewsResourceCard.
9aa7e8c to
fd91863
Compare
Summary
TopicScreeninto topic string resources.Fixes #2115
Test plan
./gradlew spotlessCheck./gradlew :feature:topic:impl:testDemoDebugUnitTest./gradlew :feature:topic:impl:lintDemoDebug :feature:topic:api:lintDemoDebug./gradlew :feature:topic:impl:connectedDemoDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.google.samples.apps.nowinandroid.feature.topic.impl.TopicScreenTest