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

Pages Editor: add rules to "Add Task to Page" functionality #7079

Merged
merged 13 commits into from
May 17, 2024

Conversation

shaunanoordin
Copy link
Member

@shaunanoordin shaunanoordin commented Apr 18, 2024

PR Overview

Part of: Pages Editor MVP project and FEM Lab super-project
Follows #7075
Staging branch URL: https://pr-7079.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging

This PR adds special rules for the "Add Task to Page" functionality. (See #7065)

The overall rule is this: a Step can only have 1 branching task (single answer question task)

(New behaviour) The specific sub-rules are:

  1. if a Step has a branching task, it can't have any other tasks.
    Screenshot: The "Add New Task to Page" button is disabled when (and only when) there's a branching ('Single'-type) Task.
    image

  2. if a Step already has at least one task, any added Question task must be a Multiple Answer Question Task.
    Screenshot: no visible changes, but clicking 'Question' will add a 'Multiple'-type Task, instead of a 'Single'-type Task
    image

  3. if a Step already has many tasks, any Multiple Answer Question Task can't be transformed into a Single Answer Question Task.
    Screenshot: 'Allow multiple' checkboxes are disabled for all Question tasks on pages with > 1 tasks.
    image

Other changes in this PR:

  • Multiple Answer Question Task (aka "Multiple" Task) has been added.
    • It shares many same code as the Single Answer Question Task (aka "Single"-type Task)
  • [design] For non-branching Steps/Pages, the "Next Step/Page now appears OUTSIDE the grey box.
    Old vs New:
    image image
  • [minor] functions in TasksPage have been refactored with better safety checks (e.g. if workflow doesn't exist, don't bother running "delete Task from workflow")

FAQ

Q: how do I visually tell the difference between a Single Answer Question Task (aka "Single"-type Task) and a Multiple Answer Question Task (aka "Multiple"-type Task) on the Pages Editor?
A: check whether the "Allow Multiple" checkbox is ticked. Also, "Single"-type Tasks allows you to choose the next page for each answer, not for the whole page.

"Single"-type Task:
image image

"Multiple"-type Task:
image image

Testing Steps

  • Go to the testing URL: https://pr-7065.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging
  • Click on "Reset" in the debug panel to start a workflow from scratch.
  • Add a new Page with 1 Question Task ("Single"-type).
    • You should not be able to add any further Tasks to this Page.
  • Click on the "Allow Multiple" button to change the Single Answer Question Task into a Multiple Answer Question Task.
    • You should now be able to add additional Tasks to this Page
  • Add a new Text Task to the page.
    • The "allow multiple" checkbox of the Question Task should now be disabled/locked
  • Add a new Question Task to the page.
    • The newly added Question Task should have its "allow multiple" checkbox ticked and disabled/locked. (i.e. you can't change it to a "Single"-type.)
  • Delete the Text Task and one Question Task.
    • The "allow multiple" button of the remaining Question Task should now be enabled again.

It's also worth running some quick basic tests to make sure the refactor works as expected:

  • Start with an empty workflow.
  • Add Tasks with new Pages.
    • Add Text Tasks,
    • Add Single Answer Question Tasks, and make sure the answers branch to different Tasks.
  • Add Tasks into existing Pages.
  • Edit Tasks in Pages.
  • Delete Tasks from Pages.
    • Delete all Tasks from a Page. This should delete the Page.
  • Delete Pages.
    • Any "next page => deleted Page" linkages should revert to the default "next page => submit".

Status

Ready for review. 👌

EDIT: ready to review from a functional standpoint Improvements I could/should add to this PR:

  • UI icons/tooltips to tell users why certain buttons are disabled. 🤔

@shaunanoordin shaunanoordin requested a review from a team April 18, 2024 18:01
@goplayoutside3 goplayoutside3 self-assigned this Apr 24, 2024
@shaunanoordin shaunanoordin force-pushed the pages-editor-pt19 branch 2 times, most recently from f3db498 to 22dcb15 Compare April 25, 2024 14:35
Base automatically changed from pages-editor-pt19 to master April 25, 2024 21:26
@coveralls
Copy link

coveralls commented Apr 25, 2024

Coverage Status

coverage: 56.955% (-0.02%) from 56.977%
when pulling de56b91 on pages-editor-pt20
into 9ca8f70 on master.

@goplayoutside3
Copy link
Contributor

Looking good! I'm able to follow all of the testing steps above at the preview link: https://pr-7079.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging (note that your Testing Steps section links to an older PR's staging, but I figured it out!)

A couple of questions for clarification:

  1. While all of the conditions listed in the Testing Steps work in the UI, what is the reasoning behind disallowing the combo of Single Answer Question + more tasks in the same step? I understand the logic behind disabling buttons when branching is involved, but I want to make sure I understand why only "a" is disallowed in the following step scenarios:
    - a) Single Answer Question + other task ❌
    - b) Multiple Answer Question + other task ✅
    - c) Multiple Answer Question + Multiple Answer Question + other task, etc ✅

    I'm willing to bet ya'll discussed the decision at length during Pages Editor meetings, so I apologize for asking for receipts 😆 but please point me to documentation if it already exists.

  2. When I add only one task (and therefore only one step) to a workflow, the dropdown below the step or the dropdown below each answer includes T0. Project teams should not be able to recursively select T0 when there's only one task, is that correct?
    Screenshot 2024-04-30 at 10 18 19 PM

@goplayoutside3
Copy link
Contributor

Revisiting my comment above because we talked through the reasoning behind "each single answer question gets its own page" during the weekly call. It was initially implemented this way to accommodate the fact that Single Answer Question tasks can branch, but Multiple Answer Question tasks cannot.

@shaunanoordin do you plan to update this PR with different rules for adding question tasks to their own page, or should I go ahead with review as is?

@shaunanoordin
Copy link
Member Author

@goplayoutside3 Thanks Delilah! 👍

re: "each single answer question gets its own page", your assessment is on point. 👌

re: "should a Page's/Answer's Next Page be able loop back to itself?" (i.e. can T0 go to T0?) That's a good question. Currently, the PFE/FEM Lab Project Builder does allow a Task T0 to set its next task as Task T0. This is true whether it's a branching/single-type Task or a non-branching Task. Does it make sense? Not really. I'll add this to the list of questions to ask, but once we get confirmation, implementing a "you can't loop back in on yourself" rule is fairly trivial.

Caveat: while a one-step loop of T0->T0 doesn't make sense, there is a case for a two-or-more-steps loop of T0->T1->T0. e.g. T0:"Do you see any more colours that haven't been identified?" Yes=> go to T1, No => submit and T1: "Type in the name of the colour you see" => go to T0

Next Steps:

  • I won't push any further commits to this PR, unless there's a major bug that needs fixing.
  • Additional functional changes will be added in a separate PR.
    • Notably, this will include "Updated 'Add New Task' rule - only one Question Task per page, you can have non-Question Tasks added" rule discussed on Monday.
  • Please go ahead with review as is. 👍 Thanks again!

@eatyourgreens
Copy link
Contributor

if a Step already has at least one task, any added Question task must be a Multiple Answer Question Task.

Sorry, just a question out of curiosity but does this stop you from building transcription workflows, which combine a drawing task and a yes/no question into a single step?

@shaunanoordin
Copy link
Member Author

Sorry, just a question out of curiosity but does this stop you from building transcription workflows, which combine a drawing task and a yes/no question into a single step?

Transcription Tasks are whole different kettle of fish: they fall into something called "Pre-defined Pages" which is something we're planning for phase 2.

Copy link
Contributor

@goplayoutside3 goplayoutside3 left a comment

Choose a reason for hiding this comment

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

Thanks for the follow up Shaun - LGTM!

@shaunanoordin shaunanoordin merged commit 229aeab into master May 17, 2024
5 checks passed
@shaunanoordin shaunanoordin deleted the pages-editor-pt20 branch May 17, 2024 21:56
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.

4 participants