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.

Sign up for GitHub

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: design updates for internal test #7129

Merged
merged 14 commits into from
Jun 27, 2024
Merged

Pages Editor: design updates for internal test #7129

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2cd5204
pages-editor-pt26: remove Safari fix which kills other browsers
shaunanoordin Jun 17, 2024
5d0a5ad
TasksPage design: Add New Task button only takes 50% width when workf…
shaunanoordin Jun 18, 2024
b42e07a
EditStepDialog design: Done button now has 33% min width
shaunanoordin Jun 18, 2024
2719866
NewTaskDialog design: center-align new Tasks button
shaunanoordin Jun 18, 2024
4a7bcf8
Design: replace light teal with darker teal
shaunanoordin Jun 18, 2024
ee6243b
Tasks: add functionality to hide/show Help fields. Add Collapse/Expan…
shaunanoordin Jun 21, 2024
4e9762a
Add collapsible Help field for Drawing and Question Tasks
shaunanoordin Jun 21, 2024
9fd1672
Lab Workflows: set certain projects to use PagesEditor
shaunanoordin Jun 21, 2024
4f000d2
Accessibility: hidden fields now use HTML hidden
shaunanoordin Jun 24, 2024
1d6b945
Accessibility: show/hide Help field button indicates change of state
shaunanoordin Jun 24, 2024
f72bb8f
Accessibility: show/hide Help field button indicates change of state
shaunanoordin Jun 24, 2024
8d1be63
Accessibility: show/hide Help field button indicates change of state
shaunanoordin Jun 24, 2024
1f9d10d
Tasks: restyle Help field label
shaunanoordin Jun 24, 2024
46db5b5
Feedback: dynamically alter aria description of show/hide help field …
shaunanoordin Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/pages/lab-pages-editor/components/TasksPage/TasksPage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ export default function TasksPage() {
aria-label="Choose starting Page"
className="flex-item workflow-starting-page"
onChange={handleChangeStartingPage}
style={(workflow?.steps?.length < 1) ? { display: 'none' } : undefined}
style={(workflow?.steps?.length < 1) ? { visibility: 'hidden' } : undefined}
value={firstStepKey}
>
<option value="">Choose starting page</option>
Expand Down
1 change: 1 addition & 0 deletions app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditStepDialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ function EditStepDialog({
key={`editTaskForm-${taskKey}`}
deleteTask={deleteTask}
enforceLimitedBranchingRule={enforceLimitedBranchingRule}
isFirstTaskInStep={index === 0}
stepHasManyTasks={stepHasManyTasks}
task={task}
taskKey={taskKey}
Expand Down
3 changes: 3 additions & 0 deletions app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/EditTaskForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const taskTypes = {
function EditTaskForm({ // It's not actually a form, but a fieldset that's part of a form.
deleteTask,
enforceLimitedBranchingRule,
isFirstTaskInStep = true,
stepHasManyTasks,
task,
taskKey,
Expand Down Expand Up @@ -56,6 +57,7 @@ function EditTaskForm({ // It's not actually a form, but a fieldset that's part
? <TaskForm
deleteTask={deleteTask}
enforceLimitedBranchingRule={enforceLimitedBranchingRule}
isFirstTaskInStep={isFirstTaskInStep}
stepHasManyTasks={stepHasManyTasks}
task={task}
taskKey={taskKey}
Expand All @@ -72,6 +74,7 @@ EditTaskForm.propTypes = {
enforceLimitedBranchingRule: PropTypes.shape({
stepHasBranch: PropTypes.bool
}),
isFirstTaskInStep: PropTypes.bool,
stepHasManyTasks: PropTypes.bool,
task: PropTypes.object,
taskKey: PropTypes.string,
Expand Down
37 changes: 31 additions & 6 deletions ...ges/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/DrawingTask.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { useEffect, useState } from 'react';
import PropTypes from 'prop-types';

import CollapseIcon from '../../../../../icons/CollapseIcon.jsx';
import DeleteIcon from '../../../../../icons/DeleteIcon.jsx';
import DrawingToolIcon from '../../../../../icons/DrawingToolIcon.jsx';
import ExpandIcon from '../../../../../icons/ExpandIcon.jsx';
import MinusIcon from '../../../../../icons/MinusIcon.jsx';
import PlusIcon from '../../../../../icons/PlusIcon.jsx';

Expand Down Expand Up @@ -51,6 +53,7 @@ const TOOL_TYPE_OPTIONS = [

function DrawingTask({
deleteTask = DEFAULT_HANDLER,
isFirstTaskInStep = true,
stepHasManyTasks = false,
task,
taskKey,
Expand Down Expand Up @@ -140,6 +143,11 @@ function DrawingTask({
return false;
}

const [ showHelpField, setShowHelpField ] = useState(isFirstTaskInStep || task?.help?.length > 0);
function toggleShowHelpField() {
setShowHelpField(!showHelpField);
}

// For inputs that don't have onBlur, update triggers automagically.
// (You can't call update() in the onChange() right after setStateValue().)
// TODO: useEffect() means update() is called on the first render, which is unnecessary. Clean this up.
Expand Down Expand Up @@ -319,14 +327,30 @@ function DrawingTask({
</span>
</div>
<div className="input-row">
<label
className="big spacing-bottom-S"
htmlFor={`task-${taskKey}-help`}
>
Help Text
</label>
<div className="flex-row spacing-bottom-S">
<label
className="medium"
htmlFor={`task-${taskKey}-help`}
Copy link
Contributor

@eatyourgreens eatyourgreens Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This requires the input to be in the DOM at all times. You can’t have a label without its corresponding input in the DOM, so the show/hide pattern used in this PR is broken. It works visually but produces a broken DOM when the text input is removed. That won’t work in a screen reader.

This needs to use a details/summary pattern, or the correct ARIA roles and state for a collapsible panel, in order to work for any users that can't physically see the task editor.

I’d also suggest using the HTML hidden boolean attribute to hide the input, rather than removing it from the DOM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the WAI-ARIA pattern for building a disclosure widget, including roles, states and keyboard interactions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, of course, you could use the native HTML disclosure widget.

Copy link
Contributor

@eatyourgreens eatyourgreens Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Scott O'Hara summarised the pros and cons of using a native disclosure widget vs. rolling your own in JavaScript. You can use that to make a decision as to which fits best here.

>
Help Text
</label>
<button
aria-label={`Show/Hide Help field`}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can modify this text to be more accurate to the existing state:

${(showHelpField) ? 'Hide' : 'Show'} Help Field

Copy link
Contributor

@eatyourgreens eatyourgreens Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That would change the labels to 'Show help field (closed)' and 'Hide help field (expanded)', since the ARIA expanded state is read out as part of the label. I don't think it's recommended practice to change the labels for toggles, based on the toggle state. Rather, have a single toggle button label that makes sense when accompanied by the open/closed state.

The visible label could be changed, though, for sighted users.

aria-controls={`task-${taskKey}-help`}
aria-expanded={showHelpField ? 'true' : 'false'}
className="plain"
onClick={toggleShowHelpField}
type="button"
>
{showHelpField
? <CollapseIcon />
: <ExpandIcon />
}
</button>
</div>
<textarea
id={`task-${taskKey}-help`}
hidden={!showHelpField}
value={help}
onBlur={update}
onChange={(e) => { setHelp(e?.target?.value) }}
Expand All @@ -338,6 +362,7 @@ function DrawingTask({

DrawingTask.propTypes = {
deleteTask: PropTypes.func,
isFirstTaskInStep: PropTypes.bool,
stepHasManyTasks: PropTypes.bool,
task: PropTypes.object,
taskKey: PropTypes.string,
Expand Down
37 changes: 31 additions & 6 deletions ...es/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/QuestionTask.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { useEffect, useState } from 'react';
import PropTypes from 'prop-types';

import CollapseIcon from '../../../../../icons/CollapseIcon.jsx';
import DeleteIcon from '../../../../../icons/DeleteIcon.jsx';
import ExpandIcon from '../../../../../icons/ExpandIcon.jsx';
import MinusIcon from '../../../../../icons/MinusIcon.jsx';
import PlusIcon from '../../../../../icons/PlusIcon.jsx';

Expand All @@ -10,6 +12,7 @@ const DEFAULT_HANDLER = () => {};
function QuestionTask({
deleteTask = DEFAULT_HANDLER,
enforceLimitedBranchingRule,
isFirstTaskInStep = true,
stepHasManyTasks = false,
task,
taskKey,
Expand Down Expand Up @@ -73,6 +76,11 @@ function QuestionTask({
return false;
}

const [ showHelpField, setShowHelpField ] = useState(isFirstTaskInStep || task?.help?.length > 0);
function toggleShowHelpField() {
setShowHelpField(!showHelpField);
}

// For inputs that don't have onBlur, update triggers automagically.
// (You can't call update() in the onChange() right after setStateValue().)
// TODO: useEffect() means update() is called on the first render, which is unnecessary. Clean this up.
Expand Down Expand Up @@ -177,14 +185,30 @@ function QuestionTask({
</ul>
</div>
<div className="input-row">
<label
className="big spacing-bottom-S"
htmlFor={`task-${taskKey}-help`}
>
Help Text
</label>
<div className="flex-row spacing-bottom-S">
<label
className="medium"
htmlFor={`task-${taskKey}-help`}
>
Help Text
</label>
<button
aria-label={`Show/Hide Help field`}
aria-controls={`task-${taskKey}-help`}
aria-expanded={showHelpField ? 'true' : 'false'}
className="plain"
onClick={toggleShowHelpField}
type="button"
>
{showHelpField
? <CollapseIcon />
: <ExpandIcon />
}
</button>
</div>
<textarea
id={`task-${taskKey}-help`}
hidden={!showHelpField}
value={help}
onBlur={update}
onChange={(e) => { setHelp(e?.target?.value) }}
Expand All @@ -199,6 +223,7 @@ QuestionTask.propTypes = {
enforceLimitedBranchingRule: PropTypes.shape({
stepHasBranch: PropTypes.bool
}),
isFirstTaskInStep: PropTypes.bool,
stepHasManyTasks: PropTypes.bool,
task: PropTypes.object,
taskKey: PropTypes.string,
Expand Down
37 changes: 31 additions & 6 deletions app/pages/lab-pages-editor/components/TasksPage/components/EditStepDialog/types/TextTask.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { useEffect, useState } from 'react';
import PropTypes from 'prop-types';

import CollapseIcon from '../../../../../icons/CollapseIcon.jsx';
import DeleteIcon from '../../../../../icons/DeleteIcon.jsx';
import ExpandIcon from '../../../../../icons/ExpandIcon.jsx';

const DEFAULT_HANDLER = () => {};

function TextTask({
deleteTask = DEFAULT_HANDLER,
isFirstTaskInStep = true,
stepHasManyTasks = false,
task,
taskKey,
Expand All @@ -31,6 +34,11 @@ function TextTask({
deleteTask(taskKey);
}

const [ showHelpField, setShowHelpField ] = useState(isFirstTaskInStep || task?.help?.length > 0);
function toggleShowHelpField() {
setShowHelpField(!showHelpField);
}

// For inputs that don't have onBlur, update triggers automagically.
// (You can't call update() in the onChange() right after setStateValue().)
// TODO: useEffect() means update() is called on the first render, which is unnecessary. Clean this up.
Expand Down Expand Up @@ -81,14 +89,30 @@ function TextTask({
</span>
</div>
<div className="input-row">
<label
className="big spacing-bottom-S"
htmlFor={`task-${taskKey}-help`}
>
Help Text
</label>
<div className="flex-row spacing-bottom-S">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd strongly consider refactoring the shared code between DrawingTask / TextTask / QuestionTask into a file called SharedTaskFields or something like that. Without your context, I could imagine it being easy to change something in one and miss the others.

<label
className="medium"
htmlFor={`task-${taskKey}-help`}
>
Help Text
</label>
<button
aria-label={`Show/Hide Help field`}
aria-controls={`task-${taskKey}-help`}
aria-expanded={showHelpField ? 'true' : 'false'}
className="plain"
onClick={toggleShowHelpField}
type="button"
>
{showHelpField
? <CollapseIcon />
: <ExpandIcon />
}
</button>
</div>
<textarea
id={`task-${taskKey}-help`}
hidden={!showHelpField}
value={help}
onBlur={update}
onChange={(e) => { setHelp(e?.target?.value) }}
Expand All @@ -100,6 +124,7 @@ function TextTask({

TextTask.propTypes = {
deleteTask: PropTypes.func,
isFirstTaskInStep: PropTypes.bool,
stepHasManyTasks: PropTypes.bool,
task: PropTypes.object,
taskKey: PropTypes.string,
Expand Down
2 changes: 1 addition & 1 deletion app/pages/lab-pages-editor/components/TasksPage/components/NewTaskDialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function NewTaskDialog({
A task is a unit of work you are asking volunteers to do.
You can ask them to answer a question or mark an image.
</p>
<div className="flex-row flex-wrap">
<div className="flex-row flex-wrap justify-center">
<button
aria-label="Add new Text Task"
className="new-task-button"
Expand Down
5 changes: 5 additions & 0 deletions app/pages/lab-pages-editor/icons/CollapseIcon.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default function CollapseIcon({ alt }) {
return (
<span className="icon fa fa-angle-up" aria-label={alt} role={!!alt ? 'img' : undefined} />
);
}
5 changes: 5 additions & 0 deletions app/pages/lab-pages-editor/icons/ExpandIcon.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default function ExpandIcon({ alt }) {
return (
<span className="icon fa fa-angle-down" aria-label={alt} role={!!alt ? 'img' : undefined} />
);
}
12 changes: 11 additions & 1 deletion app/pages/lab/workflows-table.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Link } from 'react-router';

// TEMPORARY HACK: Pages Editor
// (@shaunanoordin 2024.06.21)
const TEST_PROJECTS_THAT_SHOULD_USE_PAGES_EDITOR = ['23976']; // Production IDs.
// WARNING: doesn't differentiate between production and staging projects

const WorkflowsTable = ({
handleSetStatsCompletenessType,
handleWorkflowStatusChange,
Expand All @@ -11,6 +16,7 @@ const WorkflowsTable = ({
project,
workflows
}) => {
const shouldUsePagesEditor = TEST_PROJECTS_THAT_SHOULD_USE_PAGES_EDITOR.includes(project?.id + '')
return (
<table className="standard-table">
<thead>
Expand All @@ -31,10 +37,14 @@ const WorkflowsTable = ({
statsVisible = !workflow.configuration.stats_hidden;
}

const viewWorkflowUrl = shouldUsePagesEditor
? labPath(`/workflows/editor/${workflow.id}`)
: labPath(`/workflows/${workflow.id}`); // Default

return (
<tr key={workflow.id}>
<td>
<Link key={workflow.id} to={labPath(`/workflows/${workflow.id}`)} activeClassName="active">
<Link key={workflow.id} to={viewWorkflowUrl} activeClassName="active">
{workflow.display_name}
{' '}(#{workflow.id})
{(project.configuration && workflow.id === project.configuration.default_workflow) && (
Expand Down
12 changes: 10 additions & 2 deletions css/lab-pages-editor.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ $grey4 = #979797
$grey5 = #E2E5E9
$grey6 = #EFF2F5
$black = #000000
$teal = #00979D
$teal = #005D69 // Dark teal. Alternate is #00979D
$darkGreen = #005D69
$greenBright = #1ED359
$redBright = #E45950
Expand Down Expand Up @@ -85,7 +85,6 @@ $fontWeightBoldPlus = 700
color: $black
font-size: $fontSizeM
padding: $sizeS
-webkit-appearance: none // Remove gradient overlay in Safari

button
border-radius: $sizeXS
Expand Down Expand Up @@ -163,6 +162,9 @@ $fontWeightBoldPlus = 700
.justify-around
justify-content: space-around

.justify-center
justify-content: center

.position-relative
position: relative

Expand Down Expand Up @@ -360,6 +362,7 @@ $fontWeightBoldPlus = 700

&.done
border: 3px solid $teal
min-width: 33%
padding: $sizeS $sizeXL

&[disabled]
Expand Down Expand Up @@ -426,6 +429,11 @@ $fontWeightBoldPlus = 700
font-size: $fontSizeL
text-transform: uppercase

label.medium
display: block
font-size: $fontSizeM
text-transform: uppercase

label.narrow, span.narrow
> *
vertical-align: middle
Expand Down
Loading