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

[JN-1551] checkboxes with multiple 'other' questions #1366

Merged
merged 8 commits into from
Jan 8, 2025

Conversation

connorlbark
Copy link
Collaborator

@connorlbark connorlbark commented Jan 6, 2025

DESCRIPTION (include screenshots, and mobile screenshots for participant UX)

By default, surveyjs only allows 1 other question per checkbox group. This PR adds the ability to make checkboxes that have multiple other questions attached, with different titles for each.

I'm not in love with this approach - it adds a new "magic" way to add questions to surveys that aren't really fully surveyjs questions but have their own stable id. But aside from some hacky things like creating separate questions elsewhere and somehow rendering them inside of another question's checkboxes, I felt like this was the most elegant solution with the caveat of some weirdness if you rely on surveyjs's question list.

image image

TO TEST: (simple manual steps for confirming core behavior -- used for pre-release checks)

  • Create a participant, complete basics with an address in Massachussetts
  • in mass survey, interact with the "Schools" question
  • try multiple languages, verify that it loads the languages properly
  • Fill out the details
  • Right now, the answers for these questions don't show up on the enrollee's page, as they aren't really their own surveyjs question, so the page doesn't know about them. I think it'll be a slightly more than trivial refactor, so I'll do a cleanup PR after this to fix that if we are on board with this approach.
  • Look at data export preview, observe that the questions show up there

Comment on lines -7 to -10
demoResponse=$(curl -s 'https://admin-d2p.ddp-dev.envs.broadinstitute.org/version')
demoResponse=$(curl -s 'https://juniper-cmi.dev/version')
demoGitTag=$(echo $demoResponse | jq -r '.gitTag')

prodResponse=$(curl -s 'https://juniper.terra.bio/version')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

drive by fix

Comment on lines +165 to +172
private static String nodeToString(JsonNode node) {
if (node.getNodeType().equals(JsonNodeType.STRING)) {
return node.asText();
} else {
return node.toString();
}
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

another drive by fix - most of our survey question definitions were actually blank because if you pass a json object through the normal asText it will just be "" since it isn't 'text'. This was affecting all questions that were translated - now, the name will be, e.g. { "en": "Title", "es": ...

"name": "schools",
"type": "checkbox",
"title": "What schools did you attend?",
"renderAs": "checkbox-multiple-other",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: checkboxes are unchanged unless you include this

@connorlbark connorlbark marked this pull request as ready for review January 7, 2025 20:38
@connorlbark connorlbark changed the title wip checkbox multi detail question [JN-1551] checkboxes with multiple 'other' questions Jan 7, 2025
Copy link

sonarqubecloud bot commented Jan 7, 2025

Copy link
Collaborator

@devonbush devonbush left a comment

Choose a reason for hiding this comment

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

This looks about as clean as it could be -- especially on the javascript side

@connorlbark connorlbark merged commit baae91e into development Jan 8, 2025
19 checks passed
@connorlbark connorlbark deleted the cb-checkbox-multi-detail-question branch January 8, 2025 16:17
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