[Brand Updates] Add missing periods to Empty State messages

[hichamboushaba]

Closes: woocommerce/woomobile-private#412

Description

This PR adds periods that were missing in some empty state messages (this was requested here pffQ75-10m-p2#comment-998). I'm not entirely sure I found all the cases here, but this touches on the most important and noticeable ones.

Steps to reproduce

I think code review and screenshots are enough.

Testing information

Tested on an iPhone 16 Simulator, iOS 18.1

Screenshots

I didn't collect all screenshots as it's just text change, please let me know if you prefer me to do so:

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

• The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
• Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
• Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS
	Build Number	pr14940-834f540
	Version	21.4
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	834f540
	App Center Build	WooCommerce - Prototype Builds #12634

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[pmusolino]

LGTM, I just left a comment about not touching the Localizable.strings file

[pmusolino]

I don't think you need to remove these strings from Localizable.strings, since they will be removed automatically during the code freeze.

[hichamboushaba]

I'm not familiar with localization process for iOS, so please bear with me here 😅.

I did this because I noticed it was done before multiple times for similar cases (history of the file), if I don't update the file like this, then the old Strings defined in the file will keep showing until the localization is done, which would happen at the end of the code freeze if I'm not mistaken, and this would mean releasing the first beta build with the old strings, WDYT?

[hichamboushaba]

since they will be removed automatically during the code freeze.

There are two cases here I think, for Strings with defined keys, they won't be removed, they will be updated, and it's for them that my remark above applies.
For the others, I agree, there is no need to remove them, but since we need to remove some, I'm not sure if it's worth it to revert them 🤔.

[pmusolino]

Don't worry Hicham, it's understandable if you are not familiar with the process. 😉
Here you can find the instructions on how the localization works in WCiOS.
Basically, you don't need to touch the Localizable.string file. If you delete a string, no other work is needed. While if you update a string, be sure to update the key as well in the code. But also in this last case, you don't need to modify the Localizable.string.
TBH, I would revert the file, just because I wouldn't want this change to create problems for the automatic Localizable file update process.

[hichamboushaba]

I think something is still missing in the discussion, I already checked the above document, and it doesn't talk about the issue here: updating the content of some existing keys, from what I see, in the past other developers resorted to different processes: either update the content in the Localizable.string as well, or delete the key, if you prefer updating I can do it too, but it still means touching the file manually, I don't think created any issues in the past, as Glotpress will still download a new updated file at the end of the code freeze.

TBH, I would revert the file, just because I wouldn't want this change to create problems for the automatic Localizable file update process.

As I shared above, this will mean we'll release the beta at the start of code freeze with the old content, and it would also mean issues during the design review expected before giving the green light for the project.

[hichamboushaba]

Thanks @AliSoftware for the great insights, I don't think what you shared here is common knowledge, and at least in Android, we tend to frequently update the English copies directly in strings.xml when making just content changes (Strings that placeholders is another topic that I think most developers should be aware of), so I think we should update our docs to explicitly mention that the key|value pair should immutable once sent for translation.

mostly due to the imbalance that we send originals from trunk but pull translations into release/*

TIL, can I ask why this the process and why we don't take the originals from release/* at the start of code freeze? I always thought it was taking from release/*.

---

For this PR specifically, since we are just adding periods to some strings, which won't cause a significant change to translation, I think we should be safe, right?

[AliSoftware]

Thanks @AliSoftware for the great insights, I don't think what you shared here is common knowledge, and at least in Android, we tend to frequently update the English copies directly in strings.xml when making just content changes (Strings that placeholders is another topic that I think most developers should be aware of), so I think we should update our docs to explicitly mention that the key|value pair should immutable once sent for translation.

FWIW, I've written all those in a couple of past P2s a while ago, so an MGS search should give you some additional info; I've also made a presentation about all that during the WordPress team meetup in Porto 2022 (with slides and all), and most of that presentation applies to Woo and other products as well, in case that helps better understanding all that.

TIL, can I ask why this the process and why we don't take the originals from release/* at the start of code freeze? I always thought it was taking from release/*.

Well actually you're right, strings are also frozen before we send them to GlotPress:

• For WCiOS, since the Localizable.strings file is not supposed to be edited by developers and is instead generated using genstrings (from the content of the codebase) at the time we do the code-freeze, then the content of the en.lproj/Localizable.strings should always reflect the content of the last frozen state. So even if we later backmerge release/* into trunk and the en.lproj/Localizable.strings that got generated right after the code freeze is back-propagated to trunk, its content is always supposed to reflect what's used in the code that is in the release/* branch, because that's the only time that file gets regenerated/updated (right after code-freeze)
• For WCAndroid, while the values/strings.xml can be modified by devs manually anytime, during code-freeze we make a copy of that file into fastlane/resources/values/strings.xml, which is then the file that gets imported into GlotPress by the wpcom job. This ensures that even if the original strings.xml gets updated post-code-freeze by devs working on features, those changes won't get imported into GlotPress, only the changes that were frozen/copied into fastlane/resources/ during code-freeze will (and that file is not supposed to be changed manually either)

So in practice the strings that are sent to GlotPress are indeed the ones as they are at the beginning of the release/* branch right after the code freeze happened.

Which in theory means that this should cover the cases where a string is modified in trunk while it has already been sent to GlotPress during the current code-freeze and creation of the release/* branch: such a change in trunk should not impact GlotPress after all, at least not until the next code-freeze.

But there's still a risk of a key/string that was already previously translated into GlotPress and gets changed later but its new value is only partially translated while the key remains the same… in which case, while I think that GlotPress is able to be smart enough to mark such strings as "needing revision" when it detects the original changed, I fear there's still a risk that an old translation of the old string could still get pulled into the release/* branch and create an inconsistency…

Note

This is not true for all our products, for example Tumblr uses a different approach where they send the strings to the translators continuously from trunk because their code-freeze period is only one week and it'd be too short for the paid translators to translate too many strings in all locales in time if they were only sent on code-freeze Monday to be collected on Thursday to be ready for Friday's submission).
So there are products, like Tumblr, for which the risk is higher given the different process, and for which keeping those strings immutable is even more important (Internal ref: paaHJt-51A-p2); while for Woo at least we have less chances of this happening given the strings are only sent to translation once frozen.

That's why our policy about those cases have still always been "better safe than sorry"—especially since we encountered crashes due to mismatching placeholders in different locales/translations due to such cases in the past and encouraging to consider strings to be immutable helps put all the chances on our side to prevent this from happening again in the future.

For this PR specifically, since we are just adding periods to some strings, which won't cause a significant change to translation, I think we should be safe, right?

Yep, if the change is only a typo in the English copy that would not change the meaning / semantics of the copy (e.g. from "plan_price_subtitle" = "This plan is completely free!" to `"plan_price_subtitle" = "This plan is a paid option, but you can get 10% off the first month.") and that the typo that was present in the English copy is likely not present in the translations (as they would have self-corrected it while translating to the new locale)… then this should be all ok and not cause any trouble.

[hichamboushaba]

Thanks again Olivier 🙏

FWIW, I've written all those in a couple of past P2s a while ago

Yes, I've checked the P2 that Paolo shared above, and saw also the slides of the presentation you made (thanks btw ❤️), and unless I missed it, I didn't see any explicit mentions about the immutability point, and from my day to day experience, I can tell you the other Android devs are also not aware of this (maybe iOS devs too based on the git history 😅), and that's why I suggested we update our documentation to state this explicitly, I'll try to find some time to handle this for Android.

[AliSoftware]

Yeah I think at the time I wrote the slides of the presentation (and also at the time some of the past P2s were written) we didn't yet have the rule of thumb to consider strings immutable; it's mostly when we encountered the crashers later—plus seeing how it was an even bigger problem with some other product's processes (like Tumblr)—that we started to enforce this.

@iangmaia I think there have been discussions as well about introducing a Danger check to warn if developers were modifying an existing key as part of a PR? At least it could be an idea to consider. Not sure how easy it'd be—for iOS it'd mostly be detecting manual changes in the en.lproj/Localizable.strings on any PR other than the code-freeze one, for Android it'd be a bit trickier to detect changes in existing keys (even if the corresponding lines in the strings.xml were also moved to a different line in the file and not just changed in place) all while not detecting additions of new strings as false-positives…—but I'm sure there are ideas to experiment with…

[iangmaia]

I do recall some ideas around strings immutability. Tthere's this issue already created, but it seems like something else. I think I'll just update it to consider these other possible scenarios as well.