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(app): copy QA from August 17 #13344

Merged
merged 12 commits into from
Sep 14, 2023
Merged

Conversation

ecormany
Copy link
Contributor

@ecormany ecormany commented Aug 18, 2023

Overview

This PR contains a number of localization changes (and corresponding tests) found in a copy QA session done on Flex hardware on August 17. The goal is to have in-app copy match designs in Figma as exactly as possible (without significant refactoring for very small gains, like capitalization only).

The changes here are strings that I could easily find and was reasonably confident only applied to the screens we were looking at on-device. Some more complicated changes that may require refactoring will be ticketed separately.

Partially addresses RUXD-1061.

Test Plan

Check that these strings render properly in the touchscreen app.
We've already verified the new text against what's in Figma.

Changelog

  • string changes across about a dozen localization and organism files
  • copy-paste text to corresponding tests

Review requests

  1. Verify the new text appears in the appropriate places in the touchscreen app.
  2. Make sure we haven't polluted the desktop app with text that only applies to touchscreen use.
  3. Make sure we haven't broken anything else across either app due to highly shared strings.

Risk assessment

Low. This barely touches code, but there's always the possibility of breaking an interface element or otherwise worsening the user experience.

@ecormany ecormany requested a review from b-cooper August 18, 2023 14:23
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #13344 (bbc3df0) into chore_release-7.0.0 (b4430d9) will increase coverage by 0.09%.
The diff coverage is 75.00%.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13344      +/-   ##
=======================================================
+ Coverage                71.30%   71.39%   +0.09%     
=======================================================
  Files                     2420     2418       -2     
  Lines                    68076    67920     -156     
  Branches                  7907     7899       -8     
=======================================================
- Hits                     48539    48489      -50     
+ Misses                   17685    17580     -105     
+ Partials                  1852     1851       -1     
Flag Coverage Δ
app 68.92% <75.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
app/src/organisms/GripperWizardFlows/MovePin.tsx 58.06% <ø> (ø)
...rc/organisms/GripperWizardFlows/UnmountGripper.tsx 62.96% <ø> (ø)
...isms/RobotSettingsDashboard/RobotSystemVersion.tsx 100.00% <ø> (ø)
...p/src/organisms/LabwarePositionCheck/CheckItem.tsx 70.73% <50.00%> (-0.52%) ⬇️
...p/src/organisms/LabwarePositionCheck/PickUpTip.tsx 60.00% <100.00%> (+1.26%) ⬆️

... and 34 files with indirect coverage changes

@@ -14,8 +14,8 @@
"confirm_position_and_move": "Confirm position, move to slot {{next_slot}}",
"confirm_position_and_pick_up_tip": "Confirm position, pick up tip",
"confirm_position_and_return_tip": "Confirm position, return tip to Slot {{next_slot}} and home",
"ensure_nozzle_is_above_tip": "Ensure that the pipette nozzle furthest from you is centered above and level with the top of the tip in the A1 position. If it isn't, use the controls below or your keyboard to jog the pipette until it is properly aligned.",
"ensure_tip_is_above_well": "Ensure that the pipette tip furthest from you is centered above and level with well A1 on the labware. If it isn't, use the controls below or your keyboard to jog the pipette until it is properly aligned.",
"ensure_nozzle_is_above_tip": "Ensure that the pipette nozzle furthest from you is centered above and level with the top of the tip in the A1 position. If it isn't, tap <bold>Move pipette</bold> and then jog the pipette until it is properly aligned.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These might be trouble if they're shared between app and touchscreen. If so, refactor to use old text in app and new on touchscreen.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are shared between desktop and the touchscreen. We can add some branching logic so that the previous text shows on Desktop, and your new revision only shows on the Touchscreen if that's what you'd prefer?

@@ -33,8 +33,8 @@
"discovery_timeout": "Discovery timed out.",
"download_update": "Download app update",
"enable_dev_tools_description": "Enabling this setting opens Developer Tools on app launch, enables additional logging and gives access to feature flags.",
"enable_dev_tools": "Enable Developer Tools",
"error_boundary_description": "You need to restart your robot. Then download the robot logs from the Opentrons App and send them to [email protected] for assistance.",
"enable_dev_tools": "Developer Tools",
Copy link
Contributor

Choose a reason for hiding this comment

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

This change has an impact on Desktop app.
Do we want to change Desktop app and Touchscreen app both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that's good. I find settings that start with "Enable" to be redundant and harder to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, thanks!

@b-cooper b-cooper marked this pull request as ready for review August 28, 2023 18:13
@b-cooper b-cooper requested a review from a team as a code owner August 28, 2023 18:13
@ecormany ecormany requested a review from koji September 6, 2023 15:27
@koji
Copy link
Contributor

koji commented Sep 6, 2023

@ecormany
Shlok told me that we decided to allow users to use more than 17 characters for a robot name.
Have you heard that?

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for updating copies!

@ecormany ecormany merged commit 9bcbecb into chore_release-7.0.0 Sep 14, 2023
22 checks passed
@b-cooper b-cooper deleted the copy-review-aug17 branch October 5, 2023 17:22
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.

3 participants