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

Subscribers Page: Add strings for translation #77807

Closed

Conversation

ivan-ottinger
Copy link
Contributor

@ivan-ottinger ivan-ottinger commented Jun 5, 2023

⚠️ Please do not merge this PR. ⚠️

Resolves #77676.

TODO:

  • add screenshots to this PR before merging
Subscribers page Subscription details "Grow your subscribers" page
Markup on 2023-06-06 at 11:08:31 Markup on 2023-06-06 at 11:10:05 Markup on 2023-06-06 at 11:12:09

Proposed Changes

The PR introduces strings that are related to the:

  • Subscribers page
  • "Grow your subscribers" section and page
  • "Subscription details" page (even though this is not part of the Milestone I., I decided to include the strings in case there will be some changes needed related to handling single / multiple paid subscriptions per user)

The PR includes also strings like:

  • Strings for "confirmation dialogs" - messages that the site owner will see when they click on the "Remove" button.
  • "Manage" button label (not in the design, but we may need it)
  • "Open rate" string (and similar) that aren't planned to be delivered in Milestone I

ℹ️ The PR does not include strings related to the subscribers import screens.

Testing Instructions

  1. Review the strings, check for typos and compare them with the strings in the design (h8tMpJlCqNFemEerU9owHI-fi-1737_56094).
  2. Make sure the PR doesn't introduce any regressions.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

@ivan-ottinger ivan-ottinger added the Subscribers Page The new Subscribers page in Calypso label Jun 5, 2023
@ivan-ottinger ivan-ottinger self-assigned this Jun 5, 2023
@github-actions
Copy link

github-actions bot commented Jun 5, 2023

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@ivan-ottinger ivan-ottinger requested a review from a team June 5, 2023 16:08
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 5, 2023
Comment on lines +68 to +76
// Confirmation dialogs
const removeFreeSubscriberConfirmation = translate(
'Are you sure you want to remove %s from your subscribers? They will no longer receive emails from you.',
{ args: [ username ], comment: "%s is the subscriber's username" }
);
const removePaidSubscriberConfirmation = translate(
'Are you sure you want to remove %s from your subscribers? They will no longer receive emails from you. You will not be able to add them back as a paid subscriber.',
{ args: [ username ], comment: "%s is the subscriber's username" }
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would you say about these @yansern? 🙂 (I am pinging you here, since we were chatting about these confirmation dialogs earlier a bit)

@simison
Copy link
Member

simison commented Jun 5, 2023

You know you can use "string freeze" label to send these for translating without merging?

@ivan-ottinger
Copy link
Contributor Author

You know you can use "string freeze" label to send these for translating without merging?

Thank you, Mikael!

I am aware about the "string freeze" label, but didn't think about using it here actually - as once the strings are reviewed, I was planning to just merge the PR.

Would you say that it would be better to just get the strings translated without merging this PR and then once the strings are merged by their own PRs (when the related features are introduced), just drop this temporary PR?

Copy link
Contributor

@phcp phcp left a comment

Choose a reason for hiding this comment

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

Regarding Subscribers page I noticed it's missing two strings (as highlighted on the image below). It's also missing some strings a few strings from Subscriber details, but I didn't added them here since it seems not to be intended to cover this part on the PR.
image

Not sure what other folks in the team think, but I agree with Mikael that we could use "string freeze" to avoid merging temporary code.

@ivan-ottinger
Copy link
Contributor Author

Thank you for the review, Paulo! ☺️

Regarding Subscribers page I noticed it's missing two strings (as highlighted on the image below).

I missed the tooltip string and added it now in fa95c44.

The "Total" string is already translated elsewhere, so I won't add it in this PR (it was just recently introduced in the WIP design and still may change).

It's also missing some strings a few strings from Subscriber details, but I didn't added them here since it seems not to be intended to cover this part on the PR.

Yeah, these are being changed rapidly at the moment so I am not going to add them for now and leave only the strings from the original design.

Going to apply the string freeze label.

@ivan-ottinger ivan-ottinger added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Jun 7, 2023
@a8ci18n
Copy link

a8ci18n commented Jun 7, 2023

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/8028804

Thank you @ivan-ottinger for including a screenshot in the description! This is really helpful for our translators.

@a8ci18n
Copy link

a8ci18n commented Jun 15, 2023

Translation for this Pull Request has now been finished.

@ivan-ottinger
Copy link
Contributor Author

This PR served its purpose of triggering the translation of strings included and is no longer necessary. → Closing it.

@github-actions github-actions bot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Subscribers Page The new Subscribers page in Calypso
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subscribers Page: Create and commit temporary file with all strings that need translation
5 participants