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

Safe markdown: consolidate delete account strings #30969

Merged
merged 6 commits into from Sep 30, 2019

Conversation

nkiruka
Copy link
Contributor

@nkiruka nkiruka commented Sep 26, 2019

The objective is to consolidate delete account strings to improve translation experience.

Implementation

  • Six strings were consolidated into two strings.
  • SafeMarkdown component is used to render formatted text instead of raw text.

Screen shots

  • Before

Screen Shot 2019-09-25 at 5 16 24 PM

  • After

Screen Shot 2019-09-25 at 5 16 45 PM

@nkiruka
Copy link
Contributor Author

nkiruka commented Sep 26, 2019

@dju90 I haven't found a way to open a markdown link in a new tab. From my findings, the default is still using html. This will impact consolidating the strings. Do we still want to open the link in a new tab (this seems to be a common best practice)?

Current: link opens in a new tab
before_change

New implementation: link does not open in a new tab
after_change

@nkiruka
Copy link
Contributor Author

nkiruka commented Sep 26, 2019

markdown link reference

@dju90
Copy link
Contributor

dju90 commented Sep 26, 2019

Since it goes to a *.code.org domain, I would be fine not opening it in a new tab. I think standard is to open in a new tab only if it goes to an external site (and sometimes not even then), so we're good here to open in the same tab.

@Hamms
Copy link
Contributor

Hamms commented Sep 26, 2019

The recommended standard is to open links in a new tab if and only if opening them in the current tab would cause the user to lose some progress; if the current tab contains a form, for example, that the user might be in the process of filling out but which is not saved.

"deleteAccount_teacherWarning5": "sending home instructions for creating personal logins. ",
"deleteAccount_teacherWarning6": "Give them at least a few days to follow these instructions BEFORE you delete your account.",
"deleteAccount_teacherWarning1": "Deleting your account will permanently erase all personal information, coursework, projects, and professional learning information connected to this account after 28 days. **It will also delete your sections and your students’ accounts** that don’t have a personal login or aren’t in another teacher’s section. Please make sure you have the authority to delete these students’ education records before deleting your own account. ",
"deleteAccount_teacherWarning2": "Give these students a chance to keep using their Code.org accounts by [sending home instructions for creating personal logins.]({explanationUrl}) Give them at least a few days to follow these instructions BEFORE you delete your account.",
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to create these as new strings, so the old translations aren't carried over

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also take this opportunity to give these strings more descriptive names

@nkiruka nkiruka requested a review from Hamms September 27, 2019 22:57
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

Oh, test failure is legit; the SafeMarkdown components should not be in <p> tags; they'll provide paragraphs of their own

@nkiruka nkiruka merged commit f46262a into staging Sep 30, 2019
@nkiruka nkiruka deleted the consolidate-delete-acct-strings branch September 30, 2019 00:34
@dju90
Copy link
Contributor

dju90 commented Sep 30, 2019

Will this change be caught by the sync up today? If so, I can let someone on the international program know so that we can reach out to the translator who reported it :D

@nkiruka
Copy link
Contributor Author

nkiruka commented Sep 30, 2019

@dju90 Yes. I added the string to the list I shared with Jorge
https://docs.google.com/document/d/1mfqgUFUwHzSs2stcT_9DpYQO7tuiJX5_Z_2PB5lWB6U/edit

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.

None yet

3 participants