(refactor): KHP3 Add Dynamic Attribute Handling and enhance "Sync" & "Edit" Functionalities for Providers #565
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
Omoshlawi
requested changes
Jan 31, 2025
There was a problem hiding this comment.
thank @its-kios09 , seems good except for the small issues on translations
| mutate((key) => { | ||
| return typeof key === 'string' && key.startsWith('/ws/rest/v1/provider'); | ||
| }); | ||
| mutate((key) => typeof key === 'string' && key.startsWith('/ws/rest/v1/provider')); | ||
| showSnackbar({ | ||
| title: 'Success', |
There was a problem hiding this comment.
wouldnt it be better we made this translatable using trasnlation, simil to Failure titles?
| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| placeholder="License number" |
| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| placeholder="Registration number" |
donaldkibet
requested changes
Jan 31, 2025
| showSnackbar({ | ||
| title: 'Failure', | ||
| kind: 'error', | ||
| subtitle: t( | ||
| 'errorMsg', | ||
| `Error ${provider ? 'upating' : 'creating'} provider! ${error?.responseBody?.error?.message}`, | ||
| `Error ${provider ? 'updating' : 'creating'} provider! ${error?.responseBody?.error?.message}`, |
There was a problem hiding this comment.
This is not going to work well. We should be using the translate third parameter e.g
t('errorMessage' 'Error {{message}}' , {message: error.responseBody.error.message})| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| id="form__national__id" |
There was a problem hiding this comment.
Suggested change
| id="form__national__id" | |
| id="form__national__id" |
We should be following standard convention here e.g formNationalId
| {...field} | ||
| id="form__national__id" | ||
| placeholder={t('nationalIdPlaceholder', 'Enter National ID')} | ||
| labelText={t('nationalID', 'National ID*')} |
There was a problem hiding this comment.
We should leverage this pattern from carbon
| /> | ||
| )} | ||
| /> | ||
| )} | ||
|
|
||
| <Column> | ||
| <span className={styles.form__gender}>{t('gender', 'Gender*')}</span> |
There was a problem hiding this comment.
Suggested change
| <span className={styles.form__gender}>{t('gender', 'Gender*')}</span> | |
| <span className={styles.formGender}>{t('gender', 'Gender*')}</span> |
| mutate((key) => { | ||
| return typeof key === 'string' && key.startsWith('/ws/rest/v1/provider'); | ||
| }); | ||
| mutate((key) => typeof key === 'string' && key.startsWith('/ws/rest/v1/provider')); |
There was a problem hiding this comment.
Suggested change
| mutate((key) => typeof key === 'string' && key.startsWith('/ws/rest/v1/provider')); | |
| import { restBaseUrl } from '@openmrs/esm-framework'; | |
| mutate((key) => typeof key === 'string' && key.startsWith(`restBaseUrl/provider`)); |
| @@ -221,7 +220,9 @@ const ProviderForm: React.FC<ProvideModalProps> = ({ closeWorkspace, provider, u | |||
| person: personPayload, | |||
| roles: data.roles, | |||
| }; | |||
|
|
|||
| let _user; | |||
There was a problem hiding this comment.
lets avoid naming variable like this
| @@ -53,7 +57,7 @@ function CustomActionMenu({ provider }: CustomActionMenuProps) { | |||
| <OverflowMenu flipped={document?.dir === 'rtl'} aria-label="overflow-menu"> | |||
| <OverflowMenuItem itemText="Edit" onClick={() => handleUpdateProvider(provider)} /> | |||
| <MenuItemDivider /> | |||
| <OverflowMenuItem itemText="Sync" onClick={handleOpenModal} disabled={!providerNationalId} /> | |||
| <OverflowMenuItem itemText="Sync" onClick={handleOpenModal} disabled={!canSync} /> | |||
There was a problem hiding this comment.
Suggested change
| <OverflowMenuItem itemText="Sync" onClick={handleOpenModal} disabled={!canSync} /> | |
| <OverflowMenuItem itemText="Sync" onClick={handleOpenModal} disabled={!isSyncEnabled} /> |
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.
Requirements
Summary
The button is enabled only if at least one of the following provider attributes has a value:
Dynamic Provider Payload Construction:
Screenshots
None.
Related Issue
None.
Other
None.