Classifier Dropdown Task (v3) - Simple Dropdown

[shaunanoordin]

PR Overview

Package: lib-classifier
Associated issues: #1767 (ADR), #1679 (RFC)
Associated Project: Engaging Crowds

This PR adds a Simple Dropdown (type: dropdown-simple) task to the monorepo Classifier.

• The Simple Dropdown functions like a HTML <select> which allows a single value to be selected
• The Simple Dropdown is separate from the classic Dropdown (type: dropdown) from PFE
• At the moment, the dropdown-simple task type cannot be built in the PFE Project Builder, and has to be set up by a dev. Example project: https://local.zooniverse.org:8080/?env=production&project=12754&workflow=16106

Please note that Simple Dropdown and Dropdown are two separate tasks, but can coexist. This v3 branch (forward-looking Simple Dropdown, for new projects) is developed on top of the v2 branch (backwards-compatible classic Dropdown, for eventually porting PFE projects to the monorepo).

Dev Notes

Simple Dropdown Task model:

"T0":{
  "help":"",
  "type":"dropdown-simple",
  "options":[
    "Red",
    "Blue",
    ...
  ],
  "required":false,
  "allowCreate":false,
  "instruction":"Choose your favourite colour"
}

Simple Dropdown Annotation model:

{
  "task":"T0",
  "value": {
    "value": 1,  // Corresponds to "Blue"
    "option":true
  }
},

NOTE: if a simple dropdown has no value selected (as is the case when initially rendered), the annotation value is null.

{
  "task":"T0",
  "value": null,  // No value selected
}

Status

WIP

The simple dropdown is functional, annotations work, etc - but I need to clean up the README and snip out vestigial code from the v2 development.

[shaunanoordin]

PR Update

This PR is now ready for review. I've added additional notes, stating that when a Simple Dropdown Task has no value selected, it should submit null as its annotation.

I'm thinking of folding the DdSelect subcomponent into the SimpleDropdownTask main component, since the subcomponent was originally created to support multiple <select>s per dropdown task. 🤔

[srallen]

I have some initial comments and recommendations on just an initial read through of the code

[shaunanoordin]

PR Update

Thanks to @srallen for spotting the major error with SimpleDropdownAnnotation - it was recording the text value instead of the index value of selected options. Also thanks to @eatyourgreens for reinforcing the idea that the index value will be important for getting consistent answers across translations.

This branch has been updated with the following:

• SimpleDropdownAnnotation model has been updated to { selection: (number, index of the selection), option: true }
  • NOTE: SimpleDropdownAnnotation model can actually also support { selection: (string), option: false }, but this shouldn't arise in actual projects, unless we manually enable the 'Other' option in the code.
• Component cleanup: DdSelect has been removed and merged into SimpleDropdown
• README has been updated

Status

Ready for re-review!

[eatyourgreens]

Looks good to me. Can the nested ternary operators be split up? I find them very hard to read.

I'm a bit confused by the Other option. It's disabled, and has no tests, so I wonder if we should remove it from this iteration of the dropdown task?

[shaunanoordin]

Thanks Jim! The disabled 'Other' option has been detailed in the README.md, and the discussions of whether to snip the code out together was discussed in the v2 thread (sorry, that's a long haul). But the TL;DR is that the work was done and we probably will want to support it eventually, so might as well keep it as a simple toggle.

I'll have to fix those ternary operators if they're hard to read, brb.

[shaunanoordin]

👍 selectedOption has been made more readable. I'd like to also note that Sarah's CRs have been addressed in
87b7fa7, though 2055110 is probably more interesting.

These changes have been addressed.

[srallen]

One blocking issue:

• Left an inline note on the usage of the disabled prop. You can see this working with the other task stories like the single task or multiple choice task. If you use a knob to change the subject loading state to loading, then you're unable to select an option. For the simple dropdown, if I set it to loading, I can still use the dropdown select.

Then a couple of non-blocking comments:

• I can use the Select with keyboard and pick an option using enter, but not space. I'm not sure if we want it working with space, but we've been customizing other task types to work with spacebar I think? Maybe @eatyourgreens has thoughts on this.
• And yes, just for clarification, we're unsure if we're continuing to support the free text option here, but since Shaun already coded it, it has been left in with a few things still left to do like the tests. We have stats that show that a lot of project builders have enabled it, but it's unclear if it is actually being used. Analyzing classifications would be a lot of work, so that's why we don't know yet.

[srallen]

Grommet's Box doesn't take a disabled prop, so I'm not sure what this does here.

[eatyourgreens]

WebAIM says Spacebar for selecting options.
https://webaim.org/techniques/keyboard/#testing

That said, I think I usually use Enter when selecting an option with a native control, like the language menu in PFE projects.

EDIT: WebAIM says Spacebar to expand the menu. I should have read it more closely.

[eatyourgreens]

I've tested with the English/German language menu on Every Name Counts, which is a HTML <select>: Space expands the menu, cursor keys navigate the options and Enter/Space selects an option (tested on macOS.)
https://www.zooniverse.org/projects/cseidenstuecker/every-name-counts

[eatyourgreens]

I've added disabled to the select menu. One question about code organisation: should this new task be in tasks/experimental?

[eatyourgreens]

Trying this task out in the dev classifier, I'm finding that the classifier crashes when you submit a classification. #1830 (comment)

EDIT: I see the same crash with a single choice question, so it's nothing to do with the dropdown task. I think this is ok to merge.

[shaunanoordin]

PR Update

I've updated this PR to match master and performed some extra checks to ensure functionality still looks legit.

• Please note that the new testing URL is: https://local.zooniverse.org:8080/?env=production&project=12754&workflow=16106&subjectSet=85771 - the subjectSet parameter is required because WF 16106 is grouped and there's a (new(ish?)?) check that requires grouped WFs to specify the Subject Set ID
• A workflow with a simple dropdown (type: simple-dropdown) with 4 options displays correctly, and allows a user to select from the 4 options, and the Classification annotation is in the form of the index-value of the option.

Note: this PR will have #1830 merged on top of it.

Again, much thanks to Jim for fixing that disabled issue. @srallen are there any other blockers I missed?

[srallen]

I don't think I have anything else for this. LGTM. We do need to update the associated ADR with examples that are still missing for it...

I don't think this needs to be an experimental tool. This is essentially simplified / restricted version of the existing PFE task that comes as a result of known uses and lessons learned from that version. If there is an experiment that can be defined, I'd be happy to have it organizationally moved. This is in contrast to the transcription task, which is still an experiment, and I'm working with Sam right now to setup a test project re-running ASM data so we can compare the resulting classifications to make sure it works as we expect.

I had a look at https://local.zooniverse.org:8080/?env=production&project=12754&workflow=16106&subjectSet=85771 and the stories in storybook and the task appears to function as defined with tests passing.

Note: one non-blocking request would be to update the story for the task to be similar to the other task stories where we have a storybook-knob toggle to toggle the required state. I manually tested the required state by changing the task required state in the code and refreshing.

[eatyourgreens]

Shaun's on leave now so I'm gong to merge this.