OC-22077 Query widget appearing when clicking add repeat group button in form #738
Conversation
There was a problem hiding this comment.
I think this issue is caused by enketo-core calling goToTarget on the whole form/new repeat since https://github.com/enketo/enketo-core/pull/969/files#diff-ca036963ec5e89fdaa73c05b2ff5381f3f22d89c8ca700daa74aee65e0821dc8. I made another related fix (but upon form load - not repeat-related: enketo/enketo#1286).
After looking at this, I am also wondering if the bug also exists outside of repeats (e.g. a 1 question + 1 dn form where the dn question is ahead of the related question in the DOM).
public/js/src/module/form.js
Outdated
| 'input:not(.ignore):not([readonly]), textarea:not(.ignore):not([readonly]), select:not(.ignore):not([readonly])'; | ||
|
|
||
| // For repeat DOM, prevent focus on DN dome when all element is readonly or ignore(#733) | ||
| if (target.querySelector('.repeat-number')) { |
There was a problem hiding this comment.
What if the user wants to open a discrepancy note that is inside a repeat (using the goto parameter in the API - which is then added to the URL as a query parameter)? I think your fix might break this functionality. (Please test this situation in your updated PR as well)
There was a problem hiding this comment.
I think you may have to separate 2 selector options: (1) target.closest('.question') with the unchanged selector and (2) else excluding discrepancy notes. This will also fix a possible yet-undiscovered bug I mention later. So (2) would cover the cases where target is a form or repeat else (or page, or whatever else that is not a question).
I wish there was a more elegant solution, but I think you're right that overwriting goToTarget seems best (because comment widgets are not officially supported in enketo/enketo). The problem will be to keep this function up-to-date with future upstream changes. However, at least once we migrate to OpenClinica/enketo-oc (the monorepo) we could make these changes directly in enketo-core which will be helpful for things like this.
There was a problem hiding this comment.
sorry @kkrumlian not yet, I plan to focus on it tomorrow
@MartijnR
I think you are right about the goToTarget, but since it's on enketo-core, I tried to prevent making a change there afraid to make another issue. But I tried to make an update on goToTarget in enketo-core by adding another parameter to indicate isClone or not and decided not to use it.
I'll check the dn first on Dom, and for the API, I did check gotoQuery works fine with the current update.
I'll also test your suggestion and will inform you later.
Thank you.
There was a problem hiding this comment.
@MartijnR code updated, I put your suggestion and tested it.
But I found an issue when trying to open the query within the record(I think this is using API like you mention)
This error also happen before the code updated so I think it's not related with the code changes above. Do you have any clue why this happens?
While the other API call to open the query only works fine with the code above
There was a problem hiding this comment.
@theywa, I think the query parameter is not correct, so Enketo assumes the question does not exist. Here's an example I use to get the API to return a well-formed (escaped etc) query parameter (for another form):
curl --user something: -d "server_url=http://localhost:8005&form_id=Max&instance=\
<max1 xmlns:oc= \"http://openclinica.org/xforms\" oc:complete= \"true\">\
<try1>\
<hh_num>3</hh_num>\
<hh_num_comment></hh_num_comment>\
</try1>\
</max1>&instance_id=a&go_to=/max1/try1/hh_num_comment&ecid=a" \
http://localhost:8005/oc/api/v1/instance/editIt returns a URL like this: http://localhost:8005/edit/fs/i/lLUP0OfK?ecid=a&instance_id=a#%2Fmax1%2Ftry1%2Fhh_num_comment
There was a problem hiding this comment.
I didn't test it myself though...
OpenClinica/enketo-oc#20