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

Survey plugin rewrite (fixes #2758) #3204

Merged
merged 65 commits into from Apr 30, 2024
Merged

Survey plugin rewrite (fixes #2758) #3204

merged 65 commits into from Apr 30, 2024

Conversation

becky-gilbert
Copy link
Collaborator

@becky-gilbert becky-gilbert commented Jan 3, 2024

Fixes #2758

This PR rewrites the survey plugin so that it works as a wrapper for SurveyJS. Now any SurveyJS-compatible JSON can be passed directly into the plugin, which allows jsPsych users to use all SurveyJS features without our having to implement them in the plugin.

The updated SurveyJS architecture requires the survey-core package along with one of the following UI/rendering packages: react, angular, vue, knockout, jquery. I've switched from knockout to jquery because jquery seems to require the least/simplest deviations from our conventional plugin design/style.

To do:

  • Add a parameter that allows the user to define a function that manipulates the survey object in JS after it has been created (rather than just defining it in JSON - see here).
  • Move the various examples in example.html into multiple files in a new examples/ directory, to make them a little easier to understand.
  • Draft the docs update - working on it now!
  • Add basic tests for basic plugin functionality and parameters (I will assume that the SurveyJS library itself is well tested)
  • Check/fix the current survey plugin issues (see list in this comment).

Requests for feedback:

  1. I'm struggling a bit with the styling. Can anyone have a look at the example.html file and provide feedback/suggestions for layout and styling? Any feedback is helpful, but the more specific the better!
    SurveyJS has a number of pre-defined themes that you can see here: https://surveyjs.io/form-library/documentation/manage-default-themes-and-styles. I think we'll probably want to make at least a few customizations to get the style to match the rest of jsPsych's default style a little more closely. Unfortunately I can't find a comprehensive list of the SurveyJS CSS variables and their meanings, so thus far I've been their online GUI tool to make theme edits and generate the corresponding CSS variables/values, which is slow ☹️
  2. Any other suggestions for improvement would be appreciated!

Examples and screenshots

Example: checkbox/multi-select, conditional question display, and carrying answers forward to next question

  const animals = {
    title: "jsPsych Survey Plugin: referencing answers from previous questions",
    elements: [{
      type: "checkbox",
      name: "favorite-animals",
      title: "What are your favorite animals?",
      description: "Please select at least TWO features to see the Carry Forward functionality.",
      isRequired: true,
      colCount: 2,
      choices: [
        "Hippopotamus",
        "Raccoon",
        "Kangaroo",
        "Shark",
        "Cat",
        "Hedgehog",
        "Bunny",
        "Monkey"
      ]
    }, {
      type: "ranking",
      name: "animals-ranked",
      title: "Which of these animals would make the best pet? Please rank your favorite animals from BEST (1) to WORST pet option.",
      visibleIf: "{favorite-animals.length} > 1",
      isRequired: true,
      choicesFromQuestion: "favorite-animals",
      choicesFromQuestionMode: "selected"
    }],
    showQuestionNumbers: false
  };

  const animals_trial = {
    type: jsPsychSurvey,
    survey_json: JSON.stringify(animals)
  };

Rank question is hidden until at least two animals are selected in the checkbox question:

multiselect_rank_hidden

multiselect_rank_visible

Data:

multiselect_rank_data

Copy link

changeset-bot bot commented Jan 3, 2024

🦋 Changeset detected

Latest commit: e807de4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@jspsych/plugin-survey Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@DominiqueMakowski
Copy link

Just chiming in to say I'm very excited to see this PR happen, it's going to be a big improvement, so thanks for all the work and good luck for the remaining bits 💪

@becky-gilbert
Copy link
Collaborator Author

@DominiqueMakowski Thanks!! So glad to hear this will be appreciated - I agree it's going to be a big improvement. I'll finish this up ASAP, just working on the docs now.


// This example shows how to make some questions conditional on previous answers
// from the same survey trial.
const vegetables = {
Copy link
Member

Choose a reason for hiding this comment

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

Why does the location of the next button change in this demo? If I select neutral it is on the right, otherwise in the center.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah thanks for flagging this. I fixed this.
The reason was that there are actually 3 nav buttons at the bottom of the page: previous, next, and complete. And depending on the participants response, they are either done with the survey (selected neutral) or going on to another page. I fixed this by adding some CSS that takes the 'complete' button out of the page flow (display: none) when it is hidden.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also: this means that the buttons could potentially move on the page (if a response is made that changes the visibility of 'complete' without also removing the 'next' button), or the button could stay in the same place but the label would change between 'next' and 'complete'. I think this is still preferable to having weird previous/next button placement because of a 'complete' button that is in the page flow but hidden.

@jodeleeuw
Copy link
Member

hey @becky-gilbert this is awesome. left just a few minor things to adjust or consider.

Copy link
Member

@jodeleeuw jodeleeuw left a comment

Choose a reason for hiding this comment

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

Looks great!

@becky-gilbert becky-gilbert merged commit 6d99a71 into main Apr 30, 2024
4 checks passed
@github-actions github-actions bot mentioned this pull request Apr 30, 2024
Copy link
Member

@bjoluc bjoluc left a comment

Choose a reason for hiding this comment

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

Late to the party, sorry! Wanted to add a quick reminder about removing the build outputs, but saw you did that already on main 👍


StylesManager.applyTheme();
applyStyles(survey) {
// TO DO: this method of applying custom styles is deprecated, but I'm
Copy link
Member

Choose a reason for hiding this comment

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

Time to get rid of this?


StylesManager.applyTheme();
applyStyles(survey) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be private?

@bjoluc bjoluc deleted the survey-plugin-rewrite branch May 2, 2024 19:23
@becky-gilbert
Copy link
Collaborator Author

Thanks for coming back to this @bjoluc!

Wanted to add a quick reminder about removing the build outputs, but saw you did that already on main

Yeah, sorry for forgetting to do this here ☹️ I think I need to make a few other minor changes in addition to the things you've pointed out, so I'll start another PR.

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.

allow survey plugin to accept any SurveyJS-compatible JSON model
4 participants