Pages Editor: implement Branching Controls #6991

shaunanoordin · 2023-12-12T01:24:08Z

PR Overview

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

This PR adds the ability to choose the next Page, for Steps with branching Tasks.

i.e. any Step with a Single Question Task will enable branching controls.
- When branching controls are enabled, a choice of Next Step/Page is given for every answer/choice in the task
- The Next Step/Page is identified by the Tasks keys within that Step/Page.
The visual UI doesn't fully match Sean's design (notably, the yellow arrows aren't going beyond the Step body's container) but that's
- (2024.02.13 EDIT: I forgot to finish this sentence earlier. It should have said "The visual UI doesn't fully match Sean's design, but that's something I'll take care of in a future PR.")

Other changes:

Add "Preview Workflow" button (⚠️ hardcoded to WF 3711! I need to update DataManager to pull project data to get this fully working)
When dragging Steps to rearrange, the Steps now have a yellow outline

Status

Ready for review!

Do not merge until 6981 is merged. Currently targets pages-editor-pt12 for reviewing purposes; this will be re-targeted to master when ready.

app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx

coveralls · 2023-12-12T01:26:17Z

coverage: 56.976% (+0.01%) from 56.965%
when pulling 5803cf4 on pages-editor-pt13
into 0e611cf on master.

kieftrav

Overall LGTM. Small suggestions that are inline with other PR's.

kieftrav · 2023-12-12T13:33:48Z

app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx

            <StepItem
+              key={`stepItem-${step[0]}`}


camelCase vs kebab-case

kieftrav · 2023-12-12T13:35:02Z

app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx

+          const task = allTasks[taskKey];
+          return (
+            <EditTaskForm
+              key={`editTaskForm-${taskKey}`}


camelCase vs kebab

kieftrav · 2023-12-12T13:38:46Z

app/pages/lab-pages-editor/components/TasksPage/components/StepItem/BranchingControls.jsx

+  const answers = task.answers || []
+
+  function onChange(e) {
+    const next = e.target?.value;


should be e?. or e. between this line and the below line.

kieftrav · 2023-12-12T13:43:01Z

app/pages/lab-pages-editor/components/TasksPage/components/StepItem/BranchingControls.jsx

+  );
+}
+
+function NextStepArrow({


This should probably be pulled out as an Icon / it's own file so it's usable in other parts of the codebase (if needed)

kieftrav · 2023-12-12T13:48:48Z

PR looks good to me and doesn't create any weird error states. The video below shows the behavior of having empty values for answer/choices and then editing those answer/choices after you've set some branching conditions. Not sure if this is the desired behavior to delete/destroy the branching conditions upon changing answer/choices.

Also, your PR Overview has an unfinished sentence: "The visual UI doesn't fully match Sean's design (notably, the yellow arrows aren't going beyond the Step body's container) but that's" (and ends with that... What were you trying to say?

pr-review-6991.mov

shaunanoordin · 2024-02-13T15:12:09Z

Thanks Travis!

Also, your PR Overview has an unfinished sentence: "The visual UI doesn't fully match Sean's design (notably, the yellow arrows aren't going beyond the Step body's container) but that's" (and ends with that... What were you trying to say?

Holy crap, did I leave that sentence hanging?? I meant to say "visual UI doesn't fully match Sean's design, but that's something I'll take care of in a future PR. " Sorry, I should have re-read my PR overview before I posted.

Now on to the TODO List:

~~❗ 🆕 Double check behaviour where Question Task has 1. empty Answer, or 2. duplicate Answers. (Does PFE allow empty answers, or are they auto-deleted?)~~
- OK, so in PFE empty answers are valid. In the code that I built, apparently I made it so that empty answers are preserved while you're editing the task (so the input fields don't suddenly disappear if you delete all the text) but are silently removed when the workflow is saved. (Which must have made sense to Previous Shaun.) Duplicate answers are valid.
~~Fix missing noreferrer/noopener in Preview Workflow link.~~ ✅
~~(from 6981) fix accessibility issues~~
- Also fixed an issue where pressing the Enter key on the Instruction or Required fields of a Single Question Task would cause the first Answer to be deleted. Yes, seriously.
(from 6981) review code style
~~(from 6981) 🐛 ❗ Examine can't rearrange certain Pages via key issue~~
- ❓ Can't reliably replicate issue. Possible network latency/race condition?
~~(from 6973) implement DEFAULT_HANDLER~~ ✅

I'll append a few of those updates to this PR, then start on pt14 to address the rest of the feedback.

shaunanoordin · 2024-02-13T18:17:18Z

OK I'm finally ready to merge this. I've incorporated a lot of the feedback taken from 6981 and 6973. (Thanks Jim, Mark, and Travis!)

PR Update

Security: "Preview Workflow" link now has rel="noopener noreferrer"
Code improvement/Efficiency: anonymous () => {} functions in multiple files now use a const DEFAULT_HANDLER = () => {}
Accessibility: tuned up HTML for Single Question Task and Text Task.
Fix: previously, on a Single Question Task, pressing the Enter key in the Instructions text field OR the Required checkbox would cause the FIRST ANSWER to be deleted. This has been fixed by adding type=button to the Delete <button>s
- Funny thing about this! Apparently I forgot to add type=button to the Delete Answer <button>s.
- Which mean that these buttons were implicitly type=submit.
- Which meant that pressing Enter on any non-button input on the same form (e.g. the Instructions text field) would trigger the "submit" action. Which does two things: 1. tell the <form> to submit (which is why our form has an onSubmit handler to cancel any default submit action), and 2. _it presses the FIRST <button type="submit"> that it sees.
- At least on macOS + Chrome 121! That second thing is completely new to me and deserves its own sub-investigation.

Investigated, but no action taken (other than making a note that this behaviour should be documented):

re: "What if Single Question Task has blank Answers?".
- In PFE empty/blank answers are valid. (i.e. task.answers = [ { label="" } ])
- In the code that I built, apparently I made it so that empty answers are preserved while you're editing the task (so the input fields don't suddenly disappear if you delete all the text) but are silently removed when the workflow is saved. (Which must have made sense to Previous Shaun.)

Status

Merging, go!

Next for Pages Editor: either Deleting Tasks (easy?) or linking Subject Sets & Tutorials to Workflows.

shaunanoordin requested a review from a team December 12, 2023 01:24

github-advanced-security bot found potential problems Dec 12, 2023

View reviewed changes

app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx Fixed Show fixed Hide fixed

kieftrav requested review from kieftrav and removed request for a team December 12, 2023 13:23

kieftrav approved these changes Dec 12, 2023

View reviewed changes

shaunanoordin added 9 commits February 12, 2024 17:55

pages-editor-pt13 - prepare branching controls

073f0ea

Move canStepBranch into its own function

0adfda5

WIP. Add BranchingControls. Refactor how StepItem gets steps

996040e

BranchingControls: list 'next' options for each answer

c7aceb9

BranchingControls: style up

1d36d4b

BranchingControls: style next arrows

ab0b4ba

BracnhingControls: Implement branching choices

7d94dec

TasksPage: add (hardcoded) Preview Workflow button

db7dd1b

StepItem: style update. Add outline when dragging

faede1e

shaunanoordin force-pushed the pages-editor-pt13 branch from b168974 to faede1e Compare February 12, 2024 17:55

shaunanoordin added 4 commits February 13, 2024 15:44

TasksPage: fix missing rel=noopener noreferrer

c6f7703

PagesEditor: replace anonymous functions with DEFAULT_HANDLER const

1a85f0c

SingleQuestionTask: fix accessibility. Fix Form Submit (Enter key) bug

eb50b60

TextTask: fix accessibility

5803cf4

shaunanoordin merged commit 38d95c3 into master Feb 13, 2024
5 checks passed

shaunanoordin deleted the pages-editor-pt13 branch February 13, 2024 18:17

shaunanoordin mentioned this pull request Feb 16, 2024

Pages Editor: implement Delete Page action #7046

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pages Editor: implement Branching Controls #6991

Pages Editor: implement Branching Controls #6991

shaunanoordin commented Dec 12, 2023 •

edited

Loading

coveralls commented Dec 12, 2023 •

edited

Loading

kieftrav left a comment

kieftrav Dec 12, 2023

kieftrav Dec 12, 2023

kieftrav Dec 12, 2023

kieftrav Dec 12, 2023

kieftrav commented Dec 12, 2023

shaunanoordin commented Feb 13, 2024 •

edited

Loading

shaunanoordin commented Feb 13, 2024

Pages Editor: implement Branching Controls #6991

Pages Editor: implement Branching Controls #6991

Conversation

shaunanoordin commented Dec 12, 2023 • edited Loading

PR Overview

Status

coveralls commented Dec 12, 2023 • edited Loading

kieftrav left a comment

Choose a reason for hiding this comment

kieftrav Dec 12, 2023

Choose a reason for hiding this comment

kieftrav Dec 12, 2023

Choose a reason for hiding this comment

kieftrav Dec 12, 2023

Choose a reason for hiding this comment

kieftrav Dec 12, 2023

Choose a reason for hiding this comment

kieftrav commented Dec 12, 2023

shaunanoordin commented Feb 13, 2024 • edited Loading

shaunanoordin commented Feb 13, 2024

PR Update

Status

shaunanoordin commented Dec 12, 2023 •

edited

Loading

coveralls commented Dec 12, 2023 •

edited

Loading

shaunanoordin commented Feb 13, 2024 •

edited

Loading