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

[Site Intent Question] String corrections and input sanitisation #16282

Merged
merged 4 commits into from
Apr 11, 2022

Conversation

antonis
Copy link
Contributor

@antonis antonis commented Apr 8, 2022

Description

This PR:

To test:

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    Added a couple new unit tests with 8c78c8a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 8, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@antonis antonis added this to the 19.6 ❄️ milestone Apr 8, 2022
@antonis antonis marked this pull request as ready for review April 8, 2022 06:34
@antonis antonis requested a review from a team April 8, 2022 06:34
@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

Nice work Antonis! Tested and working well, and the code looks good as well. 👍

@mkevins mkevins merged commit ad8cf9e into release/19.6 Apr 11, 2022
@mkevins mkevins deleted the fix/site-intent-question-fixes branch April 11, 2022 06:50
Comment on lines -3209 to +3210
<string name="new_site_creation_intents_header_subtitle">Choose a topic from the list below or type your own</string>
<string name="new_site_creation_intents_input_hint">Eg. Fashion, Poetry, Politics</string>
<string name="new_site_creation_intents_header_subtitle">Choose a topic from the list below or type your own.</string>
<string name="new_site_creation_intents_input_hint">E.g. Fashion, Poetry, Politics</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: the wording changes won't get into GlotPress in time for 19.6, as the translation process has already started last week and strings have already been sent to our translation vendor over the weekend.

This means that the English copy will have this typo fixed in 19.6, but the translations will be based on the old English copy (though the translators might already have fixed this missing . in their translation, especially if e.g. translates to another abbreviation in their locale)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the clarification @AliSoftware 🙇

@ParaskP7
Copy link
Contributor

FYI: This fix made it to 19.6-rc-3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants