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

fix: remove duplicate language in onboarding instructions #144

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

alangsto
Copy link
Contributor

COSMO-233

A section of text in the submitted onboarding instructions was duplicated. I believe this may have been an oversight when copying over text for the instructions.

@@ -51,14 +51,6 @@ const SubmittedOnboardingExamInstructions = () => {
<p>
<FormattedMessage
id="exam.SubmittedProctoredExamInstructions.text4"
defaultMessage={'If you do not have an onboarding profile with the system, Verificient '
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense to me, but I have a concern about the ID.

Because we're removing the string with the ID exam.SubmittedProctoredExamInstructions.text5, I looked around for where it's used and where the string with ID exam.SubmittedProctoredExamInstructions.text1 is used, because it'll be the ID of the string on the page. I noticed that exam.SubmittedProctoredExamInstructions.text1 is used to identify two unique strings in this library. I'm not sure what happens when two IDs are used for two different strings - how does the Open edX translations code decide which string to send to Transifex for that ID? At least in this locale, the other string is associated with this ID. I think it would be safer to keep this string with the ID exam.SubmittedProctoredExamInstructions.text5 because it's unique. Do you see any issues with keeping this ID instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that makes a lot of sense! I will change this back to using exam.SubmittedProctoredExamInstructions.text5

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry. I swore this string had ID text5 before, but now it's text4 🫨. I think the string that we are keeping (which has ID text1 above) should have text4. text1 is associated with two different strings currently, so if we switch to text4, then only text4 will be associated with this string, right?

Copy link
Contributor Author

@alangsto alangsto Apr 16, 2024

Choose a reason for hiding this comment

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

I see what you're saying! Sorry I didn't catch that before. I will update the ID for text1 to be text4

@alangsto alangsto force-pushed the alangsto/update_onboarding_instructions branch from 59c1b68 to fec02a0 Compare April 12, 2024 14:14
@alangsto alangsto force-pushed the alangsto/update_onboarding_instructions branch from fec02a0 to dab850e Compare April 16, 2024 12:41
Copy link
Contributor

@MichaelRoytman MichaelRoytman left a comment

Choose a reason for hiding this comment

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

Thanks for bearing with me on that!

@alangsto alangsto merged commit e09a97a into main Apr 16, 2024
6 checks passed
@alangsto alangsto deleted the alangsto/update_onboarding_instructions branch April 16, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants