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: rebuild "Next Page" logic. Add placeholder answers. #7055

Merged
merged 11 commits into from Apr 8, 2024

Conversation

shaunanoordin
Copy link
Member

@shaunanoordin shaunanoordin commented Mar 11, 2024

PR Overview

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

This PR rebuilds the "Next Page" logic. Plus, it adds placeholder answers (fake text input & fake buttons for tasks). All this has been done mostly for UI examining purposes.

Next Page Logic:

  • Previously, the Branching Controls (the component that lets were the property of the Page/Step.
    • However, this was a problem - the branching controls weren't setting step.next, but instead step.task[x].answer[y].next! This meant that if we have more than 1 Single Question Task on the Page/Step, and especially if that SQT wasn't the final Task on the Page/Step, the visual design would be broken.
  • Now, we're moving "Branching Next Controls" from the Page/Step level to the Task level.
    • The branching next page controls will only appear on the FIRST Single Question Task of the Page/Step, if it has any.
    • ❗ The FEM classifier has this rule: if there are multiple Single Question Tasks on a Page/Step, then the FIRST SQTask dictates the branching logic.
  • We're also adding a "Simple Next Controls" at the Page/Step level.
    • If a Page/Step doesn't have a branching Task, then it'll have a "simple next page controls" at the end of it.

Placeholder Answers:

  • Text Tasks and Single Question Tasks (if they're not the branching task) now have placeholder answers (fake text input/buttons), as per Sean's designs.
  • This UI feature has been packaged with this PR, as it's required to visualise various wonky workflow combinations, for a design review.

Misc:

  • A new Quick Setup button has been added to demonstrate wonky workflow configurations. And by "wonky" I mean it may bork the intended UI design, but is otherwise technically functional to the FEM classifier.

Screenshot: an example workflow created by the new Quick Setup. Has 2 pages with 3 tasks. The first page has two SQTasks, but only the first has branching controls. The 2nd and 3rd tasks have fake text input/buttons to visually demonstrate how the task would looks like in the classifier. The 2nd page has simple "next page" control, currently pointing to "Submit"

image

Testing

  • Open the staging URL. You should be looking at a test workflow (e.g. staging wf 3711)
  • Click on the Quick Setup (Advanced, Branching) button in the debug menu below
  • Observe the visual layout of the workflow on the Pages Editor. Confirm that it looks readable to you.
  • Attempt to change the "Next Page" of any branching page. Confirm that the change works.
  • Attempt to change the "Next Page" of any non-branching page. Confirm that the change works.
    • ⚠️ NOTE: when re-arranging pages, pages are automatically re-linked to the next page. This is a behaviour that's still under discussion, please ignore it until further confirmation.

Status

Mon 11 Mar: WIP

Mon 18 Mar: Ready for Review

This PR is currently targeting pages-editor-pt16 for review purposes. Do not merge until 7052 is ready and this branch's merge is re-targeted to master.

@shaunanoordin
Copy link
Member Author

PR Update

New behaviour:

  • Automatic step/page linking now removed.
    • In previous designs, the "next step/page" of a non-branching step/page was implicit in the order of steps/pages.
    • Now, the next step/page has to be explicitly selected via dropdown.
  • cleanupTasksAndSteps(): now also removes orphaned references in step.next

Following our discussion on Mon 11 March 2024, a new plan for future behaviour has been noted:

  • If a Page has a branching task, it can have ONLY that branching task (single answer questions).
  • If a Page wants to have more than one task, it can have any task EXCEPT branching tasks (single answer question tasks).
    • Single answer question tasks must be converted to multiple answer question tasks, for "add new task to page" to be enabled.

Status

I think there are a few design bits that need to be cleaned up, but this PR is now generally ready for review.

@shaunanoordin shaunanoordin marked this pull request as ready for review March 18, 2024 00:15
@shaunanoordin shaunanoordin requested a review from a team March 18, 2024 00:15
@shaunanoordin
Copy link
Member Author

My apologies for putting up updates on Sunday night, but the past week has had a few debug/fixing curveballs thrown my way that borked my planned Pages Editor schedule. Please ignore any Github emails from this PR until it's your actual office hours; I'm only playing catchup this weekend because of something I promised myself.

Base automatically changed from pages-editor-pt16 to master March 18, 2024 15:27
@kieftrav kieftrav requested review from kieftrav and removed request for a team March 25, 2024 18:12
Copy link
Contributor

@kieftrav kieftrav left a comment

Choose a reason for hiding this comment

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

LGTM - everything is straightforward and works as expected!

@shaunanoordin
Copy link
Member Author

Thanks Travis! 👍

@coveralls
Copy link

Coverage Status

coverage: 56.955% (-0.03%) from 56.98%
when pulling 76ea935 on pages-editor-pt17
into a2a2ad4 on master.

@shaunanoordin shaunanoordin merged commit 04a4bbe into master Apr 8, 2024
5 checks passed
@shaunanoordin shaunanoordin deleted the pages-editor-pt17 branch April 8, 2024 16:27
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.

None yet

3 participants