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

I18n maker setup list #36787

Merged
merged 14 commits into from Sep 25, 2020
Merged

I18n maker setup list #36787

merged 14 commits into from Sep 25, 2020

Conversation

epeach
Copy link

@epeach epeach commented Sep 16, 2020

All the maker i18n things! This builds on PR:
#36689 to make the maker set-up process i18n-ed before I add the micro:bit.

This page is the 4 step process that folks go through to make sure their CP board is working correctly. There should be no functional changes and very minor styling changes. One change I made was to break up the sentence: "We couldn't detect a Circuit Playground board. Make sure your board is plugged in, and click re-detect." to be {We couldn't detect a Circuit Playground board. Make sure your board is plugged in, and click: } {re-detect}. This is because the re-detect has an onClick attached to it and breaking this up allowed each of those strings to be i18n'ed and still have re-detect have an onClick. My hope is that because "re-detect" also appears as a button at the top of the page, this will make sense when translated. If not, let me know and I'll head back to the drawing board with it. I included before and after images of this change.

Before:
Screenshot from 2020-09-16 16-13-44

After:
Screenshot from 2020-09-16 16-13-13

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@epeach epeach requested a review from a team September 16, 2020 23:23
@epeach epeach requested a review from a team as a code owner September 16, 2020 23:23
@epeach epeach marked this pull request as draft September 16, 2020 23:26
Base automatically changed from i18n-maker-setup to staging September 17, 2020 18:46
apps/src/lib/kits/maker/portScanning.js Outdated Show resolved Hide resolved
apps/src/lib/kits/maker/ui/SetupChecklist.jsx Outdated Show resolved Hide resolved
Comment on lines +289 to +291
{applabI18n.makerSetupPlugInBoardCheck()}
<a href="#" onClick={this.redetect.bind(this)}>
re-detect
{applabI18n.redetect()}
Copy link
Contributor

Choose a reason for hiding this comment

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

[incorporate these strings into 1 for i18n]

Copy link
Author

Choose a reason for hiding this comment

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

Hi there! Can you let me know which string parts you are looking at here? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

of course -- i was thinking we could condense applabI18n.makerSetupPlugInBoardCheck and applabI18n.redetect using the <SafeMarkdown/> component, but now i realize that won't work because the onClick handler here won't play nicely with markdown, so you can ignore this one 😄

apps/src/lib/kits/maker/ui/SetupGuide.jsx Outdated Show resolved Hide resolved
apps/src/lib/kits/maker/ui/SetupGuide.jsx Outdated Show resolved Hide resolved
@epeach epeach marked this pull request as ready for review September 22, 2020 17:37
Copy link
Contributor

@maddiedierker maddiedierker left a comment

Choose a reason for hiding this comment

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

thank you for getting everything into i18n -- LGTM!

@epeach
Copy link
Author

epeach commented Sep 25, 2020

@code-dot-org/i18n - I would love a quick check on this to make sure it's following best practice - thank you!

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.

Looks great! Thanks for being so deliberate with this.

FYI @dju90 that as we continue to engage with the process of i18n-izing old content, this could be a great reference PR

@epeach epeach merged commit 1aa7bca into staging Sep 25, 2020
@epeach epeach deleted the i18n-maker-setup-list branch September 25, 2020 19:54
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