refactor(protocol-designer): migrate i18n instances to use useTranslation hook #14326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jerader
force-pushed
the
pd_update-i18n
branch
from
6fd4a7b to
8e59572
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## edge #14326 +/- ##
==========================================
- Coverage 68.13% 68.05% -0.09%
==========================================
Files 2512 2505 -7
Lines 71478 71621 +143
Branches 9077 9084 +7
==========================================
+ Hits 48705 48739 +34
- Misses 20670 20772 +102
- Partials 2103 2110 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jerader
changed the title
jerader
changed the title
koji
reviewed
Jan 18, 2024
| @@ -41,7 +41,7 @@ | |||
| "react-dnd": "6.0.0", | |||
| "react-dnd-mouse-backend": "0.1.2", | |||
| "react-dom": "18.2.0", | |||
| "react-i18next": "13.5.0", | |||
| "react-i18next": "14.0.0", | |||
There was a problem hiding this comment.
I guess If 13.5.0 works for PD, it will be good to keep the same version as well as app.
Also, we could move these two packages into the top level package.json like app-shell and app-shell-odd do.
|
Seems that
|
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/BatchEditForm/NoBatchEditSharedSettings.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/BatchEditForm/BatchEditMoveLiquid.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
| </a> | ||
| <a className={styles.overlay_button} onClick={deleteLabware}> | ||
| {!isOver && <Icon className={styles.overlay_icon} name="close" />} | ||
| {i18n.t('deck.overlay.edit.delete')} | ||
| {'Delete'} |
There was a problem hiding this comment.
question
Any specific reason to pass the word directly?
There was a problem hiding this comment.
i'll fix this in a follow up pr where i remove the usage of all connect functions. I think i was lazy here and just changed it to not using i18n 😓
koji
reviewed
Jan 18, 2024
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/fields/MixFields.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/fields/PipetteField.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionModal.tsx
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
| @@ -37,37 +37,53 @@ export interface WellOrderModalProps { | |||
| firstValue?: WellOrderOption | null, | |||
| secondValue?: WellOrderOption | null | |||
| ) => void | |||
| t: any | |||
There was a problem hiding this comment.
oh, we need to upgrade packages...
https://www.i18next.com/overview/typescript
Could you leave a comment?
There was a problem hiding this comment.
good catch, for some reason the t was typing as any so i left it as any whenever i needed to use it as a prop. I'll link this url in the comment!
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/fields/WellSelectionField/WellSelectionField.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/forms/PauseForm.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/StepEditForm/forms/TemperatureForm.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
koji
reviewed
Jan 18, 2024
| @@ -36,21 +37,20 @@ export const ErrorContents = ( | |||
| case 'NO_TIP_ON_PIPETTE': | |||
| return ( | |||
| <> | |||
| {i18n.t(`alert.timeline.error.${props.errorType}.body1`)} | |||
| {t(`timeline.error.${props.errorType}.body1`)} | |||
There was a problem hiding this comment.
maybe deconstruct props in the future?
koji
reviewed
Jan 18, 2024
koji
reviewed
Jan 18, 2024
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/labware/BrowseLabwareModal.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/CreateFileWizard/__tests__/CreateFileWizard.test.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/CreateFileWizard/__tests__/GoBack.test.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/CreateFileWizard/__tests__/ModulesAndOtherTile.test.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/FilePipettesModal/PipetteFields.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/FilePipettesModal/index.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
protocol-designer/src/components/modals/FileUploadMessageModal/FileUploadMessageModal.tsx
Outdated
Show resolved
Hide resolved
koji
reviewed
Jan 18, 2024
| import { selectors as loadFileSelectors } from './load-file' | ||
|
|
||
| export const initialize = (store: Record<string, any>): void => { | ||
| if (process.env.NODE_ENV === 'production') { | ||
| window.onbeforeunload = (_e: unknown) => { | ||
| // NOTE: the custom text will be ignored in modern browsers | ||
| return loadFileSelectors.getHasUnsavedChanges(store.getState()) | ||
| ? i18n.t('alert.window.confirm_leave') | ||
| ? 'Are you sure you want to leave? You will lose any unsaved changes.' |
There was a problem hiding this comment.
don't need to add this text to a json file?
There was a problem hiding this comment.
i am not sure how to import the useTranslation hook here since this const is used to initialize redux. I left a comment to go back to it though. After all the connect fns are refactored out, i can do an audit of the remainder text that is not in i18n yet.
koji
reviewed
Jan 18, 2024
|
|
||
| export const getModuleShortNames = (type: ModuleType): string => { | ||
| switch (type) { | ||
| case 'heaterShakerModuleType': |
There was a problem hiding this comment.
maybe importing const from shared-data would be good?
import {
HEATERSHAKER_MODULE_TYPE,
MAGNETIC_MODULE_TYPE,
TEMPERATURE_MODULE_TYPE,
THERMOCYCLER_MODULE_TYPE
} from ''
koji
approved these changes
Jan 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes RAUT-542
Overview
This PR fixes all instances of i18n in the protocol-designer directory by using the
useTranslationhook instead. This is because the previous way we were doing it wasn't properly implementingreact-i18next.Additionally, several usages of the class components + connect function were refactored in order to use the
useTranslationhook properly (this is part of a chore that needs to be done anyway to remove all the class components)Test Plan
Smoke test PD, try to go through every flow as much as possible and call out anything that doesn't work correctly or whitescreens.
Here is the sandbox to smoke test on: https://sandbox.designer.opentrons.com/pd_update-i18n/
Changelog
i18nextandreact-i18nextdependencies to be the latestReview requests
see test plan
Risk assessment
medium-ish since it touches a bunch of code but it'll be internal