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

Fix #3384 : Arranged string resources based on screens #4823

Closed
wants to merge 181 commits into from
Closed

Fix #3384 : Arranged string resources based on screens #4823

wants to merge 181 commits into from

Conversation

MohitGupta121
Copy link
Member

@MohitGupta121 MohitGupta121 commented Jan 3, 2023

Explanation

Fix #3384 : Arranged string resources based on screens

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

  • Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
  • For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
  • Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

MohitGupta121 and others added 30 commits May 25, 2022 12:47
@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 1, 2023
@oppiabot
Copy link

oppiabot bot commented Sep 8, 2023

Hi @MohitGupta121, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 8, 2023
@oppiabot oppiabot bot closed this Sep 15, 2023
@MohitGupta121
Copy link
Member Author

@adhiamboperes PTAL, Thanks!

@MohitGupta121 MohitGupta121 reopened this Sep 26, 2023
@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 26, 2023
@MohitGupta121 MohitGupta121 marked this pull request as ready for review September 26, 2023 17:55
@oppiabot
Copy link

oppiabot bot commented Oct 3, 2023

Hi @MohitGupta121, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 3, 2023
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 3, 2023
Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Hey @MohitGupta121 , thanks for organizing this file!

In addition to my inline comments,

  • Let's resolve the activity title rename issue once it's been resolved in the discussion.
  • You do no need to make changes to the string files in the other languages, translatewiki will do the updates.
  • Please resolve the merge conflicts.

android:theme="@style/OppiaThemeWithoutActionBar" />
<activity
android:name=".app.administratorcontrols.learneranalytics.ProfileAndDeviceIdActivity"
android:label="@string/profile_and_device_id_activity_title"
android:theme="@style/OppiaThemeWithoutActionBar"/>
<activity
android:name=".app.completedstorylist.CompletedStoryListActivity"
android:label="@string/completed_story_list_activity_title"
android:label="@string/completed_story_list_activity_toolbar_title"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The label should be for the activity and not title.

android:theme="@style/OppiaThemeWithoutActionBar" />
<activity
android:name=".app.help.faq.FAQListActivity"
android:label="@string/faq_activity_title"
android:label="@string/faq_single_activity_title"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
android:label="@string/faq_single_activity_title"
android:label="@string/faq_list_activity_title"

android:label="@string/reading_text_size_activity_title"
android:label="@string/reading_text_size"
Copy link
Collaborator

@adhiamboperes adhiamboperes Oct 6, 2023

Choose a reason for hiding this comment

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

Please make sure that the activity/toolbar title strings are not confused with the label strings that are used in the options fragment. I think you will notice that you need 2 seperate strings for this.

android:windowSoftInputMode="adjustResize" />
<activity
android:name=".app.profile.PinPasswordActivity"
android:label="@string/pin_password_activity_title"
android:label="@string/pin_password_activity_pin_password_activity_label_title"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not conform to the expected pattern.

Comment on lines +3 to +4
<!-- AdministratorControlActivity -->
<string name="administrator_controls_activity_label_title">Administrator Controls</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is another instance of AdministratorControlActivity grouping on line 25, shouldn't they be together?

<string name="reveal_solution_dialog_fragment_close_icon_description">Close</string>
<!-- InputInteractionActivity -->
<string name="text_input_interaction_item_activity_text_input_default_content_description">Enter text.</string>
<string name="numeric_input_interaction_item_activity_numeric_input_hint">Enter a number.</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<string name="numeric_input_interaction_item_activity_numeric_input_hint">Enter a number.</string>
<string name="input_interaction_activity_numeric_input_hint">Enter a number.</string>

Comment on lines +209 to +242
<string name="numeric_expression_activity_default_hint_text">Type an expression here, using only numbers.</string>
<string name="algebraic_expression_activity_default_hint_text">Type an expression here.</string>
<string name="math_expression_activity_math_equation_default_hint_text">Type an equation here.</string>
<string name="math_expression_activity_error_spaces_in_numerical_input">Please remove the spaces between numbers in your answer.</string>
<string name="math_expression_activity_error_unbalanced_parentheses">Please close or remove parentheses.</string>
<string name="math_expression_activity_error_single_redundant_parentheses">Please remove the parentheses around the whole answer: \'%s\'.</string>
<string name="math_expression_activity_error_multiple_redundant_parentheses">Please remove extra parentheses around the \'(%1$s)\', for example: \'%1$s\'.</string>
<string name="math_expression_activity_error_redundant_parentheses_individual_term">Please remove the extra parentheses around \'(%1$s)\', for example: \'%1$s\'.</string>
<string name="math_expression_activity_error_unnecessary_symbols">There is an invalid \'%s\' in the answer. Please remove it.</string>
<string name="math_expression_activity_error_number_after_var_term">Please rearrange the order of %1$s &amp; %2$s. For example: %2$s%1$s.</string>
<string name="math_expression_activity_error_consecutive_binary_operators">%1$s and %2$s should be separated by a number or a variable.</string>
<string name="math_expression_activity_error_consecutive_unary_operators">Please remove the extra symbols in your answer.</string>
<string name="math_expression_activity_error_missing_lhs_for_addition_operator">Is there a number or a variable missing before or after the addition symbol \'%1$s\'? If not, please remove the extra \'%1$s\'.</string>
<string name="math_expression_activity_error_missing_lhs_for_multiplication_operator">Is there a number or a variable missing before or after the multiplication symbol \'%1$s\'? If not, please remove the extra \'%1$s\'.</string>
<string name="math_expression_activity_error_missing_lhs_for_division_operator">Is there a number or a variable missing before or after the division symbol \'%1$s\'? If not, please remove the extra \'%1$s\'.</string>
<string name="math_expression_activity_error_missing_lhs_for_exponentiation_operator">Is there a number or a variable missing before or after the exponentiation symbol \'%1$s\'? If not, please remove the extra \'%1$s\'.</string>
<string name="math_expression_activity_error_missing_rhs_for_addition_operator">There seems to be a number or a variable missing after the addition symbol \'%s\'.</string>
<string name="math_expression_activity_error_missing_rhs_for_subtraction_operator">There seems to be a number or a variable missing after the subtraction symbol \'%s\'.</string>
<string name="math_expression_activity_error_missing_rhs_for_multiplication_operator">There seems to be a number or a variable missing after the multiplication symbol \'%s\'.</string>
<string name="math_expression_activity_error_missing_rhs_for_division_operator">There seems to be a number or a variable missing after the division symbol \'%s\'.</string>
<string name="math_expression_activity_error_missing_rhs_for_exponentiation_operator">There seems to be a number or a variable missing after the exponentiation symbol \'%s\'.</string>
<string name="math_expression_activity_error_exponent_has_variable">Sorry, variables in exponents are not supported by the app. Please revise your answer.</string>
<string name="math_expression_activity_error_exponent_too_large">Sorry, powers higher than 5 are not supported by the app. Please revise your answer.</string>
<string name="math_expression_activity_error_nested_exponent">Sorry, repeated powers/exponents are not supported by the app. Please limit your answer to one power.</string>
<string name="math_expression_activity_error_hanging_square_root">Missing input for square root.</string>
<string name="math_expression_activity_error_term_divided_by_zero">Dividing by zero is invalid. Please revise your answer.</string>
<string name="math_expression_activity_error_variable_in_numeric_expression">It looks like you have entered some variables. Please make sure that your answer contains numbers only and remove any variables from your answer.</string>
<string name="math_expression_activity_error_invalid_variable">Please use the variables specified in the question and not %s.</string>
<string name="math_expression_activity_error_missing_equals">Your equation is missing an \'=\' sign.</string>
<string name="math_expression_activity_error_more_than_one_equals">Your equation contains too many \'=\' signs. It should have only one.</string>
<string name="math_expression_activity_error_hanging_equals">One of the sides of \'=\' in your equation is empty.</string>
<string name="math_expression_activity_error_unsupported_function">Function \'%s\' is not supported. Please revise your answer.</string>
<string name="math_expression_activity_error_incomplete_function_name">Did you mean sqrt? If not, please separate the variables with multiplication symbols.</string>
<string name="math_expression_activity_error_generic_text">Sorry, we couldn\'t understand your answer. Please check it to make sure there aren\'t any errors.</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I saw that these are used in the MathExpressionInteractionsViewModel, and not in an activity so they should probably revert to their original naming, and this category heading should change from MathExpressionActivity to MathExpressionInteractionsViewModel

<string name="profile_chooser_activity_set_up_multiple_profiles_description">Add up to 10 users to your account. Perfect for families and classrooms.</string>
<string name="profile_chooser_activity_profile_chooser_administrator_controls">Administrator Controls</string>
<string name="profile_chooser_activity_profile_chooser_language">Language</string>
<!-- PinPasswordActivity -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

PinPasswordActivity comes before ProfileChooserFragment and ProgressDatabaseFullDialogActivity in alphabetical order

Comment on lines +356 to +357
<string name="privacy_policy_activity_title">Policy Page</string>
<string name="policy_activity_label_title">Privacy Policy</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these two strings have been switched.

<string name="revision_card_activity_title">Skill revision page</string>
<string name="revision_card_fragment_revision_navigation_cards_header">Continue Studying</string>
<string name="bottom_sheet_options_menu_fragment_close_text">Close</string>
<!-- RecentlyPlayedActivity -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

RevisionCardActivity comes after RecentlyPlayedActivity

@oppiabot
Copy link

oppiabot bot commented Oct 6, 2023

Unassigning @adhiamboperes since the review is done.

@oppiabot
Copy link

oppiabot bot commented Oct 6, 2023

Hi @MohitGupta121, it looks like some changes were requested on this pull request by @adhiamboperes. PTAL. Thanks!

@MohitGupta121
Copy link
Member Author

  • You do no need to make changes to the string files in the other languages, translatewiki will do the updates.

@adhiamboperes Thank you so much for taking the time to review this PR.
Basically, if I change a string's name in the string.xml file then I need to also change the name of that string to other language files otherwise that string is unresolved in other languages. Right? So how do we ignore that?

@oppiabot
Copy link

oppiabot bot commented Oct 14, 2023

Hi @MohitGupta121, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 14, 2023
@MohitGupta121 MohitGupta121 removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 15, 2023
@adhiamboperes
Copy link
Collaborator

@adhiamboperes Thank you so much for taking the time to review this PR. Basically, if I change a string's name in the string.xml file then I need to also change the name of that string to other language files otherwise that string is unresolved in other languages. Right? So how do we ignore that?

@MohitGupta121, we resolved in team meeting to change the names in all translations so that we do not loose all our translations, but you need to file an issue to follow up on translatewiki to remove the "fuzzy" attribute that will likely be added.

@oppiabot
Copy link

oppiabot bot commented Oct 26, 2023

Hi @MohitGupta121, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 26, 2023
@oppiabot oppiabot bot closed this Nov 2, 2023
@adhiamboperes adhiamboperes reopened this Nov 4, 2023
@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Nov 4, 2023
Copy link

oppiabot bot commented Nov 11, 2023

Hi @MohitGupta121, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Nov 11, 2023
@oppiabot oppiabot bot closed this Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Corresponds to items that haven't seen a recent update and may be automatically closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arrange string resources based on screens
3 participants