From b19edc1325b92e59e5f971103cdff51b28d1c27b Mon Sep 17 00:00:00 2001 From: "Shaun A. Noordin" Date: Fri, 7 Jun 2024 17:58:41 +0100 Subject: [PATCH] Pages Editor: misc updates for internal review (#7105) * pages-editor-pt25: add checkIsPFEWorkflow() function * rename canStepBranch() to checkCanStepBranch() * DataManager: add PFE workflow warning * WorkflowSettingsPage: fix missing configuration.multi_image_mode dropdown * Whoops! Accidentally disabled Limited Branching Rule during a demo. * WIP: update documentation for Limited Branching Rule * WIP Limited Branching Rule: can now open New Task Dialog even if step is branching * WIP Limited Branching Rule: can now convert ONE multiple-type task (if many) into a single-type task * Limited Branching Rule (complete): newly added Question Tasks are single-type if step isn't branching, multiple-type otherwise * Limited Branching Rule: cleanup. Complete. * Next Page feature: prevent pages from linking to itself * EditStepDialog: reword Add Task button * Add better handling for Tasks that don't exist * Documentation: add comment on why deleteTask() no longer checks for task's existence * Design: increase font size of several elements on TasksPage * Add experimental container style. Makes 'next step' arrow for branching tasks appear outside the container * StepItem: fix experimental container style when there are 0 answers, or too many answers * Minor documentation * Feedback: fix gradient on select element, in Safari * Feedback: update comments on checkStepBranch() about multiple branching tasks * Feeback: TasksPage's deleteTask safety check and comments updated --- app/pages/lab-pages-editor/DataManager.jsx | 20 +- .../components/TasksPage/TasksPage.jsx | 17 +- .../EditStepDialog/EditStepDialog.jsx | 7 +- .../EditStepDialog/EditTaskForm.jsx | 28 ++- .../EditStepDialog/types/QuestionTask.jsx | 6 +- .../TasksPage/components/NewTaskDialog.jsx | 6 +- .../StepItem/BranchingNextControls.jsx | 7 +- .../components/StepItem/NextStepArrow.jsx | 2 +- .../StepItem/SimpleNextControls.jsx | 3 +- .../components/StepItem/StepItem.jsx | 174 +++++++++++------- .../components/StepItem/TaskItem.jsx | 16 +- .../WorkflowSettingsPage.jsx | 16 +- ...canStepBranch.js => checkCanStepBranch.js} | 8 +- .../helpers/checkIsPFEWorkflow.js | 17 ++ .../helpers/linkStepsInWorkflow.js | 4 +- css/lab-pages-editor.styl | 21 ++- 16 files changed, 238 insertions(+), 114 deletions(-) rename app/pages/lab-pages-editor/helpers/{canStepBranch.js => checkCanStepBranch.js} (72%) create mode 100644 app/pages/lab-pages-editor/helpers/checkIsPFEWorkflow.js diff --git a/app/pages/lab-pages-editor/DataManager.jsx b/app/pages/lab-pages-editor/DataManager.jsx index b851584765..e4f5fa06b3 100644 --- a/app/pages/lab-pages-editor/DataManager.jsx +++ b/app/pages/lab-pages-editor/DataManager.jsx @@ -17,6 +17,7 @@ import { useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import apiClient from 'panoptes-client/lib/api-client'; import { WorkflowContext } from './context.js'; +import checkIsPFEWorkflow from './helpers/checkIsPFEWorkflow.js'; function DataManager({ // key: to ensure DataManager renders FRESH (with states reset) whenever workflowId changes, use @@ -30,6 +31,7 @@ function DataManager({ status: 'ready' }); const [updateCounter, setUpdateCounter] = useState(0); // Number of updates so far, only used to trigger useMemo. + const isPFEWorkflow = checkIsPFEWorkflow(apiData.workflow); // Fetch workflow when the component loads for the first time. // See notes about 'key' prop, to ensure states are reset: @@ -129,11 +131,27 @@ function DataManager({ {apiData.status === 'error' && (
ERROR: could not fetch data
)} - {children} + {isPFEWorkflow ? : children } ); } +/* +At the moment, we don't fully support PFE workflows in the Pages Editor. +Reason: the "autocleanup" functionality (see cleanupTasksAndSteps()) would +annihilate all the tasks in a workflow with no steps (i.e. PFE workflows). +We'd need to prompt users to convert from a PFE workflow to an FEM workflow +first before allowing them + */ +function PFEWorkflowWarning() { + return ( +
+

Sorry, but the new workflow editor doesn't support traditional workflows.

+

Please check back with us later, as we improve the workflow editor.

+
+ ); +} + DataManager.propTypes = { children: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.node), diff --git a/app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx b/app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx index 1a14818b61..6061472180 100644 --- a/app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx +++ b/app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx @@ -1,6 +1,6 @@ import { useRef, useState } from 'react' import { useWorkflowContext } from '../../context.js'; -import canStepBranch from '../../helpers/canStepBranch.js'; +import checkCanStepBranch from '../../helpers/checkCanStepBranch.js'; import createStep from '../../helpers/createStep.js'; import createTask from '../../helpers/createTask.js'; import getNewStepKey from '../../helpers/getNewStepKey.js'; @@ -100,8 +100,9 @@ export default function TasksPage() { } function deleteTask(taskKey) { - // First check: does the task exist? - if (!workflow || !taskKey || !workflow?.tasks?.[taskKey]) return; + if (!workflow) return; + // NOTE: don't check if task actually exists before deleting it. + // This is useful if a Step somehow refers to a Task that doesn't exist. // Second check: is this the only task in the step? const activeStepTaskKeys = workflow.steps?.[activeStepIndex]?.[1]?.taskKeys || []; @@ -262,14 +263,12 @@ export default function TasksPage() { // Limited Branching Rule: // 0. a Step can only have 1 branching task (single answer question task) - // 1. if a Step has a branching task, it can't have any other tasks. - // 2. if a Step already has at least one task, any added question task must be a multiple answer question task. - // 3. if a Step already has many tasks, any multiple answer question task can't be transformed into a single answer question task. + // 1. if a Step has a branching task, it can't have ANOTHER BRANCHING TASK. + // 2. if a Step already has at least one BRANCHING task, any added question task must be a multiple answer question task. + // 3. if a Step already has many MULTIPLE ANSWER QUESTION tasks AND NO SINGLE ANSWER QUESTION TASK, ONLY ONE multiple answer question task CAN be transformed into a single answer question task. const activeStep = workflow?.steps?.[activeStepIndex] const enforceLimitedBranchingRule = { - stepHasBranch: !!canStepBranch(activeStep, workflow?.tasks), - stepHasOneTask: activeStep?.[1]?.taskKeys?.length > 0, - stepHasManyTasks: activeStep?.[1]?.taskKeys?.length > 1 + stepHasBranch: !!checkCanStepBranch(activeStep, workflow?.tasks) } if (!workflow) return null; diff --git a/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx b/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx index d19b74fd44..e5cfd4e2e6 100644 --- a/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx +++ b/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx @@ -99,11 +99,10 @@ function EditStepDialog({
+
+ + ); const TaskForm = taskTypes[task.type] || UnknownTask; @@ -48,9 +70,7 @@ function EditTaskForm({ // It's not actually a form, but a fieldset that's part EditTaskForm.propTypes = { deleteTask: PropTypes.func, enforceLimitedBranchingRule: PropTypes.shape({ - stepHasBranch: PropTypes.bool, - stepHasOneTask: PropTypes.bool, - stepHasManyTasks: PropTypes.bool + stepHasBranch: PropTypes.bool }), stepHasManyTasks: PropTypes.bool, task: PropTypes.object, diff --git a/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/QuestionTask.jsx b/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/QuestionTask.jsx index 7d3b98fde8..36d225e41b 100644 --- a/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/QuestionTask.jsx +++ b/app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/QuestionTask.jsx @@ -136,7 +136,7 @@ function QuestionTask({ id={`task-${taskKey}-multiple`} type="checkbox" checked={isMultiple} - disabled={enforceLimitedBranchingRule?.stepHasManyTasks && isMultiple /* If rule is enforced, you can't switch a Multi Question Task to a Single Question Task. */} + disabled={enforceLimitedBranchingRule?.stepHasBranch && isMultiple /* If rule is enforced, you can't switch a Multi Question Task to a Single Question Task. */} onChange={(e) => { setIsMultiple(!!e?.target?.checked); }} @@ -197,9 +197,7 @@ function QuestionTask({ QuestionTask.propTypes = { deleteTask: PropTypes.func, enforceLimitedBranchingRule: PropTypes.shape({ - stepHasBranch: PropTypes.bool, - stepHasOneTask: PropTypes.bool, - stepHasManyTasks: PropTypes.bool + stepHasBranch: PropTypes.bool }), stepHasManyTasks: PropTypes.bool, task: PropTypes.object, diff --git a/app/pages/lab-pages-editor/components/TasksPage/components/NewTaskDialog.jsx b/app/pages/lab-pages-editor/components/TasksPage/components/NewTaskDialog.jsx index 801f868868..037b2dbbfe 100644 --- a/app/pages/lab-pages-editor/components/TasksPage/components/NewTaskDialog.jsx +++ b/app/pages/lab-pages-editor/components/TasksPage/components/NewTaskDialog.jsx @@ -55,7 +55,7 @@ function NewTaskDialog({ // The Question Task is either a Single Answer Question Task, or a Multiple Answer Question Task. // By default, this is 'single', but under certain conditions, a new Question Task will be created as a Multiple Answer Question Task. - const questionTaskType = (!enforceLimitedBranchingRule?.stepHasOneTask) ? 'single' : 'multiple' + const questionTaskType = (!enforceLimitedBranchingRule?.stepHasBranch) ? 'single' : 'multiple' return ( {}; export default function BranchingNextControls({ allSteps = [], + stepKey, task, taskKey, updateNextStepForTaskAnswer = DEFAULT_HANDLER }) { if (!task || !taskKey) return null; - const answers = task.answers || [] + const answers = task.answers || []; + const selectableSteps = allSteps.filter(([otherStepKey]) => otherStepKey !== stepKey); function onChange(e) { const next = e.target?.value; @@ -36,7 +38,7 @@ export default function BranchingNextControls({ > Submit - {allSteps.map(([stepKey, stepBody]) => { + {selectableSteps.map(([stepKey, stepBody]) => { const taskKeys = stepBody?.taskKeys?.toString() || '(none)'; return ( - {allSteps.map(([otherStepKey, otherStepBody]) => { + {selectableSteps.map(([otherStepKey, otherStepBody]) => { const taskKeys = otherStepBody?.taskKeys?.toString() || '(none)'; return (