-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: autofill the healthcare professional info in mod #953
base: main
Are you sure you want to change the base?
Conversation
…nForm Add healthcareprofessionalsection to locales. Create healthcareprofessionalsection cypress test. Add sectionfields to healthcareProfessionalsStore. Co-authored-by: Chris Bowman <[email protected]> Co-authored-by: Karl Komeya <[email protected]> Co-authored-by: NabbeunNabi <[email protected]> Co-authored-by: NabbeunNabi <[email protected]> Co-authored-by: Matt Keighley <[email protected]>
Co-authored-by: Karl Komeya <[email protected]>
Co-authored-by: Karl Komeya <[email protected]>
Co-authored-by: Karl Komeya <[email protected]> Co-authored-by: William Brammer <[email protected]>
Co-authored-by: Chris Bowman <[email protected]>
✅ Deploy Preview for findadoc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3a12dd7
to
77ff6ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work @ShizuokaTerrier !
@@ -15,6 +20,61 @@ export const useHealthcareProfessionalsStore = defineStore( | |||
const healthcareProfessionalsData: Ref<HealthcareProfessional[]> | |||
= ref([]) | |||
const selectedHealthcareProfessionalId: Ref<string> = ref('') | |||
const selectedHealthcareProfessionalData: Ref<HealthcareProfessional | undefined> = ref() | |||
const healthcareProfessionalSectionFields = reactive({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sectionFields to me is a bit confusing. Section of what?
It's important to remember that stores are meant to be abstracted from the specific components using them.
Is this selectedHealthcareProfessional
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In combination with the initialize function it is. In that case, would a name like selectedHealthcareProfessionalEditableFields
be better? It's clear, but a bit of a mouthful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fine by me if that's accurate. I'm not sure why this one is editable and the selectedHealthcareProfessionalData
is not. (I think it should be) Can we combine these two and just make the selected one reactive? I don't see why we need to offer two different ones.
function initializeHealthcareProfessionalValues(selectedHealthcareProfessionalData: HealthcareProfessional | undefined) { | ||
if (!selectedHealthcareProfessionalData) return | ||
|
||
healthcareProfessionalSectionFields.healthcareProfessionalNameArray = selectedHealthcareProfessionalData.names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems more like a function for mapping from api data to component format rather than an initializer function. Perhaps this just needs a rename?
Also, it looks like where this is being used, we could just call this from the setSelectedHealthcareProfessional
method internally instead of from the component. Is there a use case outside of the set method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm presuming the setSelectedHealthcareProfessional
is potentially going to be used elsewhere without needing to initialize values. Is that right @NabbeunNabi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment it is only being used here to initialize the fields from the selected healthcare professional. This could be possibly used in the future. We definitely could call it when we set the selected healthcare professional. It does make sense now as @ermish is saying to call it internally
name="Name Locales" | ||
class="mb-5 px-3 py-3.5 w-96 h-12 bg-secondary-bg rounded-lg border border-primary-text-muted | ||
text-primary-text text-sm font-normal font-sans placeholder-primary-text-muted" | ||
<div v-if="isHealthcareProfessionalInitialized"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a great future issue would be to show a loader placeholder while this is being initialized! This can reduce the screen jumping all over while things are loading.
Do you want to take a shot at writing one up for us @ShizuokaTerrier ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fun @ermish!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShizuokaTerrier Did you write the ticket for this? Just curious as to whether I can resolve the conversation or not or plan on doing it this week?
Resolves #864
🔧 What changed
The selected healthcare professional's information is now autofilled.
If the professional has name info in another locale that also autofills when the locale is selected.
📸 Screenshots
Before
After
mod_edit_healthcare_professional_section_autofill.mp4