Skip to content

Move TopicScreen strings to resources#2116

Open
ois0886 wants to merge 1 commit into
android:mainfrom
ois0886:fix/topic-screen-string-resources
Open

Move TopicScreen strings to resources#2116
ois0886 wants to merge 1 commit into
android:mainfrom
ois0886:fix/topic-screen-string-resources

Conversation

@ois0886
Copy link
Copy Markdown

@ois0886 ois0886 commented May 22, 2026

Summary

  • Move the remaining hardcoded user-facing strings in TopicScreen into topic string resources.
  • Keep the visible English text unchanged while making the strings consistent with the rest of the app and ready for localization.

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

@ois0886 ois0886 requested a review from dturner as a code owner May 22, 2026 01:59
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +21 to +22
<string name="feature_topic_api_following">FOLLOWING</string>
<string name="feature_topic_api_not_following">NOT FOLLOWING</string>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
<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>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I updated the follow-state strings to sentence case in the resources.

Comment on lines +311 to +313
Text(stringResource(TopicR.string.feature_topic_api_following))
} else {
Text("NOT FOLLOWING")
Text(stringResource(TopicR.string.feature_topic_api_not_following))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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())

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

@ois0886 ois0886 force-pushed the fix/topic-screen-string-resources branch from 9aa7e8c to fd91863 Compare May 22, 2026 02:09
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.

[FR]: Move TopicScreen user-facing strings to resources

1 participant