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

SSGSO-7: Configure PRO form language #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export type SmsFormData = {
body: string;
source: string;
patientUuid: string;
locale: string;
};

export type UpdateSmsPayload = SmsFormData & {};
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,21 @@ import { first } from 'rxjs/operators';
import { useTranslation } from 'react-i18next';
import { z } from 'zod';
import { zodResolver } from '@hookform/resolvers/zod';
import { ExtensionSlot, showSnackbar, useConnectivity, useLayoutType, usePatient } from '@openmrs/esm-framework';
import {
ExtensionSlot,
showSnackbar,
useConnectivity,
useLayoutType,
usePatient,
useSession,
} from '@openmrs/esm-framework';
import { type DefaultPatientWorkspaceProps } from '@openmrs/esm-patient-common-lib';
import { type SmsFormData } from './common/types';
import styles from './send-sms-form.scss';
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually, style imports come last.

import { saveQuestionnaire } from './common';
import SendSmsField from './send-sms-input.componet';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but

Suggested change
import SendSmsField from './send-sms-input.componet';
import SendSmsField from './send-sms-input.component';

import { Select } from '@carbon/react';
import { SelectItem } from '@carbon/react';
Comment on lines +23 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

Combine these two imports. Also it's best to keep the library imports before the local file imports.


interface SendSmsFormProps extends DefaultPatientWorkspaceProps {
showPatientHeader?: boolean;
Expand All @@ -27,6 +36,7 @@ const SendSmsForm: React.FC<SendSmsFormProps> = ({
const isTablet = useLayoutType() === 'tablet';
const isOnline = useConnectivity();
const { patientUuid, patient } = usePatient();
const { locale, allowedLocales } = useSession();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure about this. What's the goal of locale switching here? Is the list of locales definitely the same as the list of locales for OpenMRS?

const visitHeaderSlotState = useMemo(() => ({ patientUuid }), [patientUuid]);
const [isSubmitting, setIsSubmitting] = useState(false);

Expand All @@ -38,6 +48,7 @@ const SendSmsForm: React.FC<SendSmsFormProps> = ({
const phoneValidation = /^[\+]?[(]?[0-9]{3}[)]?[-\s\.]?[0-9]{3}[-\s\.]?[0-9]{4,6}$/;
return z.object({
to: z.string().regex(phoneValidation, { message: 'Invalid phone number' }),
locale: z.string().min(1, { message: 'Language selection is required' }),
});
}, []);

Expand All @@ -58,17 +69,23 @@ const SendSmsForm: React.FC<SendSmsFormProps> = ({
promptBeforeClosing(() => isDirty);
}, [isDirty, promptBeforeClosing, patient]);

useEffect(() => {
if (locale) {
methods.setValue('locale', locale);
}
}, [locale]);
Comment on lines +72 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing this, it's probably better to switch the Select to a controlled input (one where we set the value) like this. This still results in default selecting the user's locale, but won't switch locales on this form if the user changes locales.


const onSubmit = useCallback(
(data: SmsFormData, event: any) => {
if (!patientUuid) {
return;
}

setIsSubmitting(true);
const { to, locale } = data;

const { to } = data;
const guid = uuid();
const body = window.location.host.concat(`/outcomes?pid=${guid}`);
const body = window.location.host.concat(`/outcomes?pid=${guid}&locale=${locale}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wherever possible, I tend to prefer to use a URLSearchParams object to construct arguments. This way the arguments you pass in the URL are always URL encoded, which ensures that they'll behave as you expect.

const source = window.location.host;

let payload: SmsFormData = {
Expand All @@ -77,6 +94,7 @@ const SendSmsForm: React.FC<SendSmsFormProps> = ({
body: body,
source: source,
patientUuid: patientUuid,
locale: locale,
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few typos below this line that aren't part of this PR, but should be fixed.

};

const abortController = new AbortController();
Expand Down Expand Up @@ -157,6 +175,18 @@ const SendSmsForm: React.FC<SendSmsFormProps> = ({
inputFieldType="text"
inputFieldPlaceholder={t('smsReceiver', 'SMS Recipient phone number')}
/>
<div>
<Select
id="locale-select"
labelText={t('selectLanguage', 'Select Language')}
invalid={!!methods.formState.errors.locale}
invalidText={methods.formState.errors.locale?.message}
{...methods.register('locale')}>
{allowedLocales?.map((allowedLocale) => (
<SelectItem key={allowedLocale} value={allowedLocale} text={allowedLocale} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably useful to look at how we get display names in the locale-switching component

))}
</Select>
</div>
</Stack>
</div>
<ButtonSet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const payload: SmsFormData = {
body: bodyValue,
source: sourceValue,
patientUuid: mockPatient.id,
locale: '',
};

const testProps = {
Expand All @@ -36,6 +37,7 @@ describe('SendSmsForm', () => {
render(<SendSmsForm {...testProps} />);

expect(screen.getByText(/Phone number/i)).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Select Language/i })).toBeInTheDocument();
expect(screen.getByRole('button', { name: /Send SMS/i })).toBeInTheDocument();
});

Expand Down
Loading