Feature/deseng657: Implemented language selector in engagement creation wizard #2560
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2560 +/- ##
=======================================
Coverage 75.96% 75.96%
=======================================
Files 609 609
Lines 21960 21960
Branches 1711 1711
=======================================
Hits 16682 16682
Misses 5015 5015
Partials 263 263
Flags with carried forward coverage won't be shown. Click here to find out more. |
| <Grid id="language-column3" columns={12} lg={4} xs={12} direction="column"> | ||
| <Await resolve={languages}> | ||
| {(languageResponse) => { | ||
| return handleLanguageColumn(languageResponse, 10, 14); |
There was a problem hiding this comment.
You may be able to revise this to remove the 15 column limit :)
There was a problem hiding this comment.
Yes I did this, and refactored the code so there is no repetition.
| @@ -149,7 +178,31 @@ const EngagementCreationWizard = () => { | |||
| All engagements must be offered in English, but you may also add content in additional | |||
| languages if you select multi-language. | |||
| </BodyText> | |||
| {_TemporaryConstructionNotice} | |||
| <Suspense fallback={<MidScreenLoader />}> | |||
There was a problem hiding this comment.
Consider using a <Skeleton> component for consistency with the rest of the app! 90vh is quite tall
| @@ -49,6 +51,7 @@ const EngagementCreationWizard = () => { | |||
| const [open, setOpen] = React.useState(true); | |||
| const navigate = useNavigate(); | |||
| const fetcher = useFetcher(); | |||
| const { languages } = useRouteLoaderData('wizard-loader') as { languages: Language[] }; | |||
There was a problem hiding this comment.
Since we're on the route we're loading from, consider using useLoaderData() instead :)
| <> | ||
| <span> | ||
| <Checkbox | ||
| defaultChecked={'English' === language.name && true} |
There was a problem hiding this comment.
| defaultChecked={'English' === language.name && true} | |
| defaultChecked={Boolean('English' === language.name)} |
Are these && true statements doing anything?
There was a problem hiding this comment.
Yes they work, but I don't mind doing it the other way
… misc fixes as per Nat's review.
|
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).