Small text edits & commented out the tract feedback button and FAQ page

[EmmaLiTsai]

This PR:

• removes the second paragraph in the "Tribal Nations" note
• removes the sentence "The Council on Environmental Quality chose them based on relevance, quality, and availability" in the methodology paragraph
• removes the link to the instructions to federal agencies on using CEJST in the methodology page
• commented out the census tract feedback survey from the side panel when a tract is selected
• commented out the FAQ Q&A text and replaced it with "coming soon!" until further notice

[titaniumbones]

thanks @EmmaLiTsai ! Looks like some unit tests are failing -- they are likely screenshot checks, which are probably solved by taking new screenshots, but I don't know exactly how to do that. @Mr0grog updated them for the previous build, and may be able to quickly explain it. I'll tag KK as a reviewer as soon as she joins the repo.

[Mr0grog]

On the command line, when running tests, use the --updateSnapshot option. That is, from the root of this repo run the following in your terminal:

cd client
npm test -- --updateSnapshot

That should leave you with a bunch of changes to files named <whatever>.test.jsx.snap. Give them a quick once-over to make sure the changes match what you expected, and commit them to git.

Then you should be good to go.

[Mr0grog]

(FWIW, I feel your pain. I’d normally try to avoid setting up snapshot testing so that it breaks on every day copy changes like this. But I don’t know enough about the structure of things in this repo or the history of the tests here — there might be a good reason they did it this way.)

[titaniumbones]

@EmmaLiTsai are you able to update the snapshots as Rob suggested? If not let me know and I will try.

[titaniumbones]

@EmmaLiTsai I had a little ADHD problem this morning & perseverated on this; if you pull my changes ot your local branch you will see that I updated the snapshots. there are some yellow unit tests when I run npm test but I imagine those were mostly already there.

the translations tests still fail, not sure if that is expected behaviour -- do you know, @Mr0grog ?

[EmmaLiTsai]

Thanks for updating the snapshots, Matt! I'm at a conference this week in California but looking at this now - I'm wondering if this is a flag that the spanish translation for the updated text is missing for the FAQ page? I commented out the Q&A section of the page in my PR but it seems that change might not have made it to the translated version...

[Mr0grog]

I'm wondering if this is a flag that the spanish translation for the updated text is missing for the FAQ page?

The error message makes it look like it’s the other way around, but yeah:

UNUSED: These keys in es.json are not in en.json

It looks like using —updateSnapshot also deleted unused English strings in @titaniumbones’s commit (surprising!) and now there’s no use for the equivalent Spanish.

I wonder why that’s happening. Since the code is just being commented (not removed) I’d think we want to keep the relevant translations. And I wouldn’t have expected —updateSnapshot to touch them in the first place…

[KameronKerger]

LGTM

[Mr0grog]

I had some time in front of a keyboard to take a look at this in detail, and if I start with @EmmaLiTsai’s latest commit to run the tests with --updateSnapshot, it doesn’t change en.json. @titaniumbones, any change you just happened to have changes to that file in your local copy — not created by the test script — and accidentally committed them?

If so, we can just roll that part back and everything should be good here.

[titaniumbones]

I will check.

…

On Tue, Feb 4, 2025 at 7:53 PM Rob Brackett ***@***.***> wrote: I had some time in front of a keyboard to take a look at this in detail, and if I start with @EmmaLiTsai <https://github.com/EmmaLiTsai>’s latest commit to run the tests with --updateSnapshot, it doesn’t change en.json. @titaniumbones <https://github.com/titaniumbones>, any change you just happened to have changes to that file in your local copy — not created by the test script — and accidentally committed them? If so, we can just roll that part back and everything should be good here. — Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACY6NDWPM322AQS2V4TCR32OFOKDAVCNFSM6AAAAABWID3IB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZVGQ3DIMBTGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[titaniumbones]

I can't figure it out but yes it must have been me. Interestingly, the previous commit that touches that file was your --updateSnapshot run, so it seems likely to me that something touches it... sometimes :-/

I suck at rolling back commits. Are you able to force-push your branch, or whatever?

[Mr0grog]

the previous commit that touches that file was your --updateSnapshot run, so it seems likely to me that something touches it... sometimes :-/

Ugh ugh. Wish I had time to build a better sense of what all is happening in this repo.

I suck at rolling back commits. Are you able to force-push your branch, or whatever?

Yep, will do shortly

[Mr0grog]

Ohhh what a mess. There is a pre-commit hook that that changes and adds that file to your commit in the middle of committing, when you have no chance to review it.

[Mr0grog]

OK, I’ve run out of time for this today, but here is what’s going on as I understand it so far:

1. In the React components, there is a <FormattedMessage> component (or intl.formatMessage() function for use outside JSX code) you can use to represent translatable text strings. When you use it, the English text goes directly in the code (in the defaultMessage prop).

2. The intl:extract script (run it manually with npm run intl:extract) finds all the <FormattedMessage> components in the code and writes the English strings from them to the file client/src/intl/en.json, effectively adding new strings that that have shown up in the code and removing any that have been removed (or commented out; as far as I can tell this tool does not attempt to look at the comments).

3. A git hook runs the above script when you commit.

4. Then tests check to make sure there aren’t any missing Spanish translations (in client/src/intl/es.json) for newly added strings. It also checks the opposite: that there aren’t old Spanish translations hanging around where the original English has been removed (the problem here). This second isn’t a serious problem (I don’t think), it’s mainly reducing bloat (usually no need to keep old translations around that are never going to be used, or to waste time sending them to the browser) and keeping you from mistakenly adding a new string with the same name but different English text and not being alerted that the old Spanish hanging around (that will suddenly start being used) isn’t a correct translation for the new English text.

So… I’m not sure what the state of things is here with translations. Are we keeping the Spanish up to date?

If yes:

• This thing is doing somewhat useful work and you probably want to follow what it’s trying to tell you by deleting the relevant lines from es.json, although you’ll have to do more work to go retrieve the old translations if/when you uncomment the English text in the site’s code.
• We’ll probably want to figure out what the right workflow for handling/updating translations is here (unless we want every PR to do both the work of drafting in English and getting the translations updated all in one).

If no, maybe we just turn this whole thing off. The Spanish translation file will get a little cruddier over time, but if we are not keeping the Spanish up-to-date, that’s just what you get. BUT I or someone probably needs to do some checking to make sure the Spanish view doesn’t just break entirely with errors if strings are missing from the Spanish file.

[EmmaLiTsai]

Hi @Mr0grog! Circling back here after touching base briefly with @titaniumbones. What do you think about turning the build cilent / translation check off for now? I don't think the larger group has decided how we want to handle the Spanish translation issue, but we can always turn the check back on in the future once we land on a strategy.

I haven't had to do this before, but I'm assuming it would just be commenting out lines 104 - 121, and 130 on the .github/workflows/build-client.yml file - let me know what y'all think and I can try that later today!

[Mr0grog]

@EmmaLiTsai That makes sense!

I'm assuming it would just be commenting out lines 104 - 121, and 130 on the .github/workflows/build-client.yml file

That would work, but I went ahead and did something fancier, under the assumption that one day we’ll want a nicer translation workflow, so we want to at least keep apprised of issues:

• If a PR changes text but not src/i18n/en.json, it’ll auto-update the file and make a little comment on the PR about it (this might happen if you edit from the GitHub UI, where the script that does this automatically doesn’t normally run, instead of from the command line on your computer). Example: Don't block the build for translation errors #19 (comment)

  

• If translations are out-of-sync between English and Spanish, it won’t stop the build, but it will log a nice message in the workflow summary. Example (from this PR): https://github.com/edgi-govdata-archiving/j40-cejst-2/actions/runs/13466017242

  

[Mr0grog]

Realized after taking a break that the same approach could be applied to the snapshot tests, so you should not need to remember to run them now if you are making changes to the text.

The actions workflows will now:

1. Check if there should have been changes made to client/src/intl/en.json and add them if needed. Also comment on the PR if changes were made so people know:

   

2. If Spanish and English text are out of sync, log a warning on the action summary, but not consider it a failure. (You have to go to the workflow summary page to see this, so it’s not too in-your-face.)

3. If changes were made to the text, update the test snapshots and post a comment about it (since updating the snapshots may allow changes unrelated to the text to proceed when they should have errored):

   

[Mr0grog]

Marking this approved so someone else can just go ahead and merge if these changes are good and help make things easier.

I can also split out the auto-update stuff from this set of changes. Just say the word!