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

Placeholder text support for level short/long instructions and authored hints #43575

Merged
merged 21 commits into from
Nov 18, 2021

Conversation

smusoke
Copy link
Contributor

@smusoke smusoke commented Nov 12, 2021

What:
This PR was worked on simultaneously and merged its branch into this one. Giving us authored_hint support for (virtually)free!

  • Updated sync-in.rb to parse short_instructions, long_instructions, and authored_hints for placeholder text block's.
  • Added the short/long instruction & authored hint placeholder text translation support into the blockly model.
  • Unit test for blockly.rb.

Why: This will allow for placeholder text blocks to be integrated into the translation pipeline and translated for these level properties moving forward.

Links

Testing story

  • Locally tested that placeholder text within short/long instructions and authored hints should be included during sync-in
  • Ran short test script that called the new localized_blockly_in_text method on every level's relevant markdown property.
Level.all.each do |l|
  if l.is_a? Blockly
    l.localized_short_instructions
    l.localized_long_instructions
    l.localized_authored_hints
  end
end
  • Created unit test for blockly model
  • Locally tested dummy placeholder translations

Screen Shot 2021-11-17 at 12 34 22 PM

Deployment strategy

Merge after #43573

Follow-up work

Similar updates to provide support for validation text.
https://codedotorg.atlassian.net/browse/FND-1771

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • 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

@smusoke smusoke requested a review from a team as a code owner November 12, 2021 19:24
@smusoke smusoke requested a review from a team November 15, 2021 22:34
Copy link
Member

@daynew daynew left a comment

Choose a reason for hiding this comment

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

Looks good, just a minor comment about testing.

@@ -567,6 +567,49 @@ def create_custom_block(name, pool, block_text, args, category: 'Events')
assert_equal expected_localized_block_xml, localized_block_xml
end

test 'localized_markdown_with_placeholder_texts' do
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it would be too much hassle to add long_instructions and short_instructions to the level we are creating in this test, and then call them to verify that those properties are actually get translated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added 👍

…translations' into fnd-1769-hints-show-translated-behav-blocks
@smusoke smusoke changed the title Placeholder text support for level short and long instructions Placeholder text support for level short/long instructions and authored hints Nov 17, 2021
Copy link
Member

@daynew daynew left a comment

Choose a reason for hiding this comment

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

Looks even better now!

@smusoke smusoke merged commit 7b4d491 into staging Nov 18, 2021
@smusoke smusoke deleted the fnd-1771-print-text-placeholder-translations branch November 18, 2021 14:50
daynew added a commit that referenced this pull request Nov 19, 2021
…xt-placeholder-translations"

This reverts commit 7b4d491, reversing
changes made to 6296140.
snickell pushed a commit that referenced this pull request Feb 3, 2024
…holder-translations

Placeholder text support for level short/long instructions and authored hints
snickell pushed a commit that referenced this pull request Feb 3, 2024
…xt-placeholder-translations"

This reverts commit 7b4d491, reversing
changes made to 6296140.
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

2 participants