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

Hai 750 #254

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Hai 750 #254

wants to merge 7 commits into from

Conversation

Dutervil
Copy link
Contributor

Please, take a look in this PR Please @mogoodrich , @dmdesimone

Copy link
Member

@mogoodrich mogoodrich left a comment

Choose a reason for hiding this comment

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

So I made two changes (removing the one line and adding another) and got it close to working... it works, but you have to manually click on the last address field to get things to "jump" ahead.

Here's the long-winded description, hope this helps:

This functionality was originally building using a "one-question-per-screen" navigator that was designed years ago (like in 2013) that was written before jquery (no mind more advanced tools).

You can find the code here:
https://github.com/openmrs/openmrs-module-uicommons/tree/master/omod/src/main/webapp/resources/scripts/navigator

Basically, it parses the HTML to come up build up a kind of section/question/field module and then allows the users to navigate through it using the keyboard and provides functionality to jump from one question to the next.

You can see some of the API methods it provides here:

https://github.com/openmrs/openmrs-module-uicommons/blob/master/omod/src/main/webapp/resources/scripts/navigator/navigator.js

In order to debug what was happening, I looked at what was happening in the personAddressWithHierachy.gsp, and specifically how it populated the fields from the "quick entry" shortcut. It was there that I noticed this line:

https://github.com/openmrs/openmrs-module-registrationapp/blob/master/omod/src/main/webapp/resources/scripts/field/personAddressWithHierarchy.js#L75

which lead to the line I suggested in the PR.

It "sort-of" works now, though the users has to still manually click through all the fields.

I suspect we can make it better by further understanding how personAddressWithHierarchy.js works and making some further changes... can you take a look and let me know what you think, @Dutervil ? Feel free to ask any further questions!

jq('#question_address').on('change', function() {
if (this.checked) {
//disable other input if the want to use the patient address as contact address
isInputDisabled(true);
Copy link
Member

Choose a reason for hiding this comment

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

Can you try removing this line? I think disabling the inputs result it in not getting saved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean @mogoodrich line 434?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, exactly @Dutervil

// target the contact address input and set the new value
jq("#contactQuestionLabel div input[type='text']").each(function(index) {
if (index < valuesArray.length) {
jq(this).val(valuesArray[index]);
Copy link
Member

@mogoodrich mogoodrich Aug 11, 2023

Choose a reason for hiding this comment

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

I added a line: " jq(this).data('legalValues', [ valuesArray[index] ])" which I think needs to happen for the entry to be considered "valid" (see my overall comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added focused class to the autocomplete when the checkbox is checked. I hoped that would help. it stil does not save in DB

@Dutervil
Copy link
Contributor Author

Dutervil commented Aug 11, 2023 via email

@Dutervil
Copy link
Contributor Author

Hi @mogoodrich . I remoced the line you said. I use this line " jq(this).data('legalValues', [ valuesArray[index] ])" that you provide. it does not work. I leave it as previous, always no adress info saved in DB. So i looked at this, but I do not see something from it that can help in that case. So what do you suggest @mogoodrich ? I would like to finish this ticket today. it lasted too long. https://github.com/openmrs/openmrs-module-registrationapp/blob/master/omod/src/main/webapp/resources/scripts/field/personAddressWithHierarchy.js#L75

@mogoodrich
Copy link
Member

This actually worked for me @Dutervil ! I checked out your latest changes and removed the comment from line 462.

Here's the screenshots I'm seeing

Here's the initial page:

2023-08-15_09-01

Then when I click on the link:
2023-08-15_09-01_1

I then have to manually click down to the last field... this was the one thing I couldn't get working properly:
2023-08-15_09-01_2

After saving, when I go to the registration summary I see the contact person address:
2023-08-15_09-02

Note if you are looking in the database, this information (and the place of birth) is stored as a obs on the registration table:

2023-08-15_09-11

@Dutervil
Copy link
Contributor Author

This is good. I will try to make the last field clicked by itself to see if this will work as expected @mogoodrich

@mogoodrich
Copy link
Member

Thanks @Dutervil ... let me know if this is ready for review.

@louidorjp
Copy link
Member

Hi @mogoodrich and @Dutervil , I've made a few modifications. It seems to be working well - you can test it now.

Copy link
Member

@mogoodrich mogoodrich left a comment

Choose a reason for hiding this comment

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

thanks @Dutervil @louidorjp , LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants