search

episphere / questionnaire

1 stars 2 forks source link

Piped text in SITTINGA was piped in and should not have been based on logic #299

Closed cusackjm closed 5 months ago

cusackjm commented 8 months ago

WORK= 0 and I still saw this piped text at SITTINGA image

image

joshid-ims commented 7 months ago

@danielruss, is this functionality (displayif within grid text) available? I am getting junk text when I try to render it.

danielruss commented 7 months ago

I think grid rows have displayif functionality. let me check...

danielruss commented 7 months ago

Here is an example

[AGE] hi how old are you |__|

|grid?|id=gridid|Directions for the page and a general introduction to the question|[ [ROW1QUESTIONID,displayif=_value('AGE')>40]  You are over 40 ;[ROW2QUESTIONID,displayif=_value('AGE')>50] You are over 50; [ROW3QUESTIONID,displayif=_value('AGE')>60] You are over 60;
[ROW4QUESTIONID] You always see this;]]|(1:response 1 text) (2:response 2 text) (3:repsonse 3 text)|
joshid-ims commented 7 months ago

In your example whole row is displayed or not based on the displayif, right? But here we want something like:

|grid?|id="GRID_SITTING"|Typical hours per DAY|[

[SITTINGA] Driving or sitting in a car, bus or train. |displayif=equals("WORK",1)|This includes commuting to and from work.|; [SITTINGB] Sitting and watching television shows, movies, or other video content on a television, tablet, phone, or computer; [SITTINGC] Sitting and browsing the internet, playing video games, using social media, or using any other apps or programs on a tablet, phone, computer, or television; [SITTINGD] Other sitting outside of work (such as for reading, crafts, and hobbies);]|

danielruss commented 7 months ago

No. you cannot have an embedded display if inside the grid. it breaks the grid markdown

|grid|params|text|questions|responses|

joshid-ims commented 7 months ago

@cusackjm, what should we do in this case?

cusackjm commented 7 months ago

it has to have the piped text skip logic. so, if it can't go in the grid, we need to pull the question out of the grid.

danielruss commented 7 months ago

@cusackjm Ok I have a fix. but it is very brittle. I can re-design the grids, but that will take time. Can you find out how important this is to be in a grid?

cusackjm commented 7 months ago

@danielruss The piped text is important because it won't make sense for people who don't work, but it is fine to just pull the question out of the grid and have it as a standalone question. That way you don't have to redesign grids

danielruss commented 7 months ago

@cusackjm -- ok I could not help myself. Before we can think about upgrading the grids, I need to update the version of bootstrap and pull out JQuery. Because this is a significant change, I asked Tony to take a look at my changes before committing to main.

Once that is done, we can take another look at the grids.

danielruss commented 7 months ago

@cusackjm please take a look at my new mockup for grids . You should be able to test it on your phone, tablet or computer. If you change the size of the window it will swap from grid to regular questions without any javascript.

All-in-all, where the new design may not be perfect, it is significantly better than the old design. One limitation is that you can only have 12 grid columns. But honestly there is not enough real estate on the screen for that many columns.

cusackjm commented 7 months ago

@danielruss that looks good in chrome. I did test in edge and the window size does not eventually swap from the grid to regular question. Not sure if that is a problem

cusackjm commented 7 months ago

@danielruss do you have an update on this?

danielruss commented 7 months ago

updated:

|grid?|id="GRID_SITTING"|Typical hours per DAY|[

[SITTINGA] Driving or sitting in a car, bus or train. %displayif=equals("WORK",1)%<i>(This includes commuting to and from work.</i>)%;
[SITTINGB] Sitting and watching television shows, movies, or other video content on a television, tablet, phone, or computer;
[SITTINGC] Sitting and browsing the internet, playing video games, using social media, or using any other apps or programs on a tablet, phone, computer, or television; [SITTINGD] Other sitting outside of work (such as for reading, crafts, and hobbies);]|(1:response 1 text) (2:response 2 text) (3:repsonse 3 text)|

[END] Thats all folks...

Notice that the displayif works exact the same as if not in the grid except that you use "%" instead if "|"

boyd-mj commented 7 months ago

@danielruss is this ready for @joshid-ims to test?

danielruss commented 7 months ago

yes

boyd-mj commented 7 months ago

Thanks! Julie, since this is ready for testing, do you want us to include this change as part of the Jan prod push?

cusackjm commented 7 months ago

@boyd-mj yes please

joshid-ims commented 7 months ago

@danielruss This original issue seems to work now but the WORKACT grid is displaying when WORK=0.

Is skipping not supported in grids? like:

|grid?|id="GRID_WORKACT",displayif=equals(WORK,1)|

or tried:

|grid?,displayif=equals(WORK,1)|id="GRID_WORKACT"|

Not sure how this was not noticed earlier.

danielruss commented 7 months ago

Hi Deepti, We have run into some issues and had to roll back to a previous version (in the PWA, the dev tool should be ok). Daniel

From: joshid-ims @.> Date: Wednesday, November 29, 2023 at 9:45 AM To: episphere/questionnaire @.> Cc: Russ, Daniel (NIH/NCI) [E] @.>, Mention @.> Subject: [EXTERNAL] Re: [episphere/questionnaire] Piped text in SITTINGA was piped in and should not have been based on logic (Issue #299)

@danielrusshttps://github.com/danielruss This original issue seems to work now but the WORKACT grid is displaying when WORK=0.

Is skipping not supported in grids? like:

|grid?|id="GRID_WORKACT",displayif=equals(WORK,1)|

or tried:

|grid?,displayif=equals(WORK,1)|id="GRID_WORKACT"|

Not sure how this was not noticed earlier.

— Reply to this email directly, view it on GitHubhttps://github.com/episphere/questionnaire/issues/299#issuecomment-1832032697, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA4X263DQNXBSUKPVUEXHE3YG5C7NAVCNFSM6AAAAAA64QEDJKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZSGAZTENRZG4. You are receiving this because you were mentioned.Message ID: @.***> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and are confident the content is safe.

joshid-ims commented 7 months ago

So should skipping in grid work in https://episphere.github.io/quest/?

joshid-ims commented 7 months ago

Issues in grid are because of the roll back as per Daniel

boyd-mj commented 6 months ago

@cusackjm @joshid-ims any updates on this? This appears to be our only outstanding issue for Module 2. Thanks!

joshid-ims commented 6 months ago

We can test once the changes are on Quest Production.

joshid-ims commented 6 months ago

One simple displayif is not working and grid gets displayed.

|grid?|id="GRID_WORKACT",displayif=valueIsOneOf("WORK","1")|

Tried 1 without " " also.

It was equals(WORK,1) before. Then tried valueEquals also. Nothing worked and grid displays even when WORK= 0.

danielruss commented 6 months ago

Remove the comma between id="..." and displayif. The questions below work as expected. 

[WORK] Do you work
(1) yes
(0) no
(2) sometimes

|grid|id=testgrid displayif=valueIsOneOf('WORK',1,2)|How Much time do you spend:| [
[STANDING] standing;
[SITTING] sitting; |
(1:some) (2:quite a bit) (3:a lot)|

[END] Good bye
joshid-ims commented 6 months ago

Thanks!

joshid-ims commented 6 months ago

"D_613744428": "1" WORKACT did not display. Piped text did not display in SITTING and the following grid. Piped text does not display in HOUSE1. Piped text does not display in LAWN1. Piped text DOES display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

"D_613744428": "0" WORKACT did not display. Piped text did not display in SITTING and the following grid. Piped text does not display in HOUSE1. Piped text does not display in LAWN1. Piped text does not display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

"D_613744428": "353358909" WORKACT did display. Piped text displayed in SITTING and the following grid. Piped text displayed in HOUSE1. Piped text displayed in LAWN1. Piped text does not display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

"D_613744428": "104430631" WORKACT did not display. Piped text did not display in SITTING and the following grid. Piped text does not display in HOUSE1. Piped text does not display in LAWN1. Piped text does not display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

FrogGirl1123 commented 6 months ago

@joshid-ims Hi Deepti, One of the things that might be adding further confusion is that the version of Quest in the renderer and the version of Quest that Connect uses may not be the same. There were some issues with the Nov. 24 release of Quest updates and these were rolled back for Connect on Nov. 28. We are meeting with Jen and other members of your team to discuss IMS testing in the PWA. So I think it would be best to pause testing until we have this discussion so you're not trying to fix issues that may or may not occur in the PWA.

joshid-ims commented 6 months ago

Sounds good. Thanks!

cusackjm commented 6 months ago

@danielruss is this ready for testing? Tony mentioned a 'piped text' issue was fixed to Aileen, but we otherwise don't have any details

joshid-ims commented 6 months ago

Of Gwen's 4 test cases, this is the relevant one:

"D_613744428": "353358909" WORKACT did display. Piped text displayed in SITTING and the following grid. Piped text displayed in HOUSE1. Piped text displayed in LAWN1. Piped text does not display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

The following code in LAWN3A, LAWN4A, LAWN4B, LAWN3C, and LAWN4C is not working after the conversion:

original: displayif=equals(isDefined(WORK,1),1)

converted: displayif=equals(isDefined(D_613744428,353358909),1)

kirksnyder commented 6 months ago

Of Gwen's 4 test cases, this is the relevant one:

"D_613744428": "353358909" WORKACT did display. Piped text displayed in SITTING and the following grid. Piped text displayed in HOUSE1. Piped text displayed in LAWN1. Piped text does not display in LAWN3A, LAWN4A, LAWN3B, LAWN4B, LAWN3C and LAWN4C.

The following code in LAWN3A, LAWN4A, LAWN4B, LAWN3C, and LAWN4C is not working after the conversion:

original: displayif=equals(isDefined(WORK,1),1)

converted: displayif=equals(isDefined(D_613744428,353358909),1)

The 1 isn't being converted to a concept ID. This issue is entirely on the IMS side. This needs to either be fixed in the markdown or in the transformer.

joshid-ims commented 6 months ago

Made changes and we are testing this now.

FrogGirl1123 commented 6 months ago

Quest version 11 is now the same version being used by prod and by the renderer so testing and fixing in the renderer should be OK for the Jan updates.

cusackjm commented 5 months ago

confirmed fix in dev testing (laptop, chrome) on 1/4/2024 (https://nih.app.box.com/integrations/officeonline/openOfficeOnline?fileId=1405598519478&sharedAccessCode=)