tenants2
tenants2 copied to clipboard
[WIP] Internationalize the Letter of Complaint front-end
This PR internationalizes the entire Letter of Complaint front end. As part of this PR, I've also internationalized our full Onboarding flow that get's used within the LOC flow. See #1652 for details on that work.
TO DO:
- [x] Figure out how to internationalize the
MIN_DAYS_TEXT
snippet from the access-dates-validation.json file in the common-data directory. The issue here is that this piece of text gets used in both the frontend (access-dates.tsx) as well as the backend (loc/forms.py). - [ ] Figure out: do we need to internationalize the LOC sample props and sample page within letter-content.tsx?
- [x] Internationalize (or figure out a different solution for) the "X characters remaining" text for inputting custom issues
- [ ] While it's not actually part of the front-end, we need to make sure we actually localize the LOC reminder we send to users 24 hours after they've signed up with LOC intent, but before they've sent a letter!
@toolness this should be ready for your review! If you can, I'd love it if you could take a look at the tests that are failing— all seem to be manifestations of the same error:
console.error
Error: Uncaught [ReferenceError: maxRecipients is not defined]
The errors started showing up after I internationalized the "email this XXX to a trusted friend" feature that shows up on the LOC confirmation page, as well as in other places. As far as I know, the only relevant change is that I added Trans tags around this bit of text that includes the maxRecipients
variable:
<Trans>
You can use the form below if you'd like us to email your{" "}
{noun} to up to {maxRecipients} addresses.
</Trans>
and maxRecipients is a common data object imported at the top of the file:
import { maxRecipients } from "../../../common-data/email-attachment-validation.json";
I was able to find a potentially related problem logged as an issue for lingui ( https://github.com/lingui/js-lingui/issues/514 ), but it still seemed odd that this was popping up now. I thought at this stage we would've run into this error before?
Thanks! I'm not sure what's going on there--it's especially weird/annoying that this problem only crops up in Jest and nowhere else--but I ended up using the workaround mentioned in the lingui issue and that seems to fix things.
Regarding the MIN_DAYS_TEXT
internationalization: I think that instead of using that, we should just commit to always specifying the minimum days in terms of either days or weeks, and then use that number in both back-end and front-end internationalized text. So in other words, the messages to localize go from e.g. First access date (at least ${MIN_DAYS_TEXT} from today)
to either ``First access date (at least ${MIN_DAYS} days from today)or
First access date (at least ${MIN_WEEKS} weeks from today)`. What do you think?
Yep, that definitely makes sense. In that case, it seems like the most sustainable option there is to commit to using days as our unit. Otherwise, if we ever need to switch to an amount of time that isn't representable through a count of weeks, we would be stuck.
On Tue, Sep 1, 2020 at 4:55 PM Atul Varma [email protected] wrote:
Regarding the MIN_DAYS_TEXT internationalization: I think that instead of using that, we should just commit to always specifying the minimum days in terms of either days or weeks, and then use that number in both back-end and front-end internationalized text. So in other words, the messages to localize go from e.g. First access date (at least ${MIN_DAYS_TEXT} from today) to either ``First access date (at least ${MIN_DAYS} days from today)orFirst access date (at least ${MIN_WEEKS} weeks from today)`. What do you think?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JustFixNYC/tenants2/pull/1649#issuecomment-685127239, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADB5OD3WYF5LHVBLTCV63DDSDVNUJANCNFSM4QKTVRPQ .
Noice yeah that sounds reasonable to me. I guess we should run it by the content folks just to make sure they are OK with it.
Ok just got the 👍 from analisa that it's fine to use days across the board!
Ok, I parceled out bits of this PR that aren't relevant to the content audit (which is what is blocking us from merging this PR) into #1656 and #1657 and merged those into master
, which will give them some time to bake in production, make this PR less massive, and reduce merge conflict headaches down the road.
Oh, nice! Good call @toolness.