fix(advanced checklist): animations and error handling (#1200)

* chore: lint * fix: task completion persisted before animation ends * fix: open/completed task transition timing * fix: save draft * fix: transitions * fix: do not show generic error when first rendering a note Co-authored-by: Johnny Almonte <johnny243@users.noreply.github.com>
2022-07-04 13:10:41 -04:00
parent 68e0acecca
commit a0205a5c7d
10 changed files with 187 additions and 146 deletions
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/app/hooks.ts
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/app/hooks.ts
@@ -1,5 +1,5 @@
-import React, { useEffect, useRef, useState } from 'react'
 import useResizeObserver from '@react-hook/resize-observer'
+import React, { useEffect, useRef, useState } from 'react'
 import { TypedUseSelectorHook, useDispatch, useSelector } from 'react-redux'
 import type { AppDispatch, RootState } from './store'

@@ -42,3 +42,13 @@ export const useDebouncedCallback = (callback: () => void, waitMs: number = 500)
    callback()
  }, waitMs)
 }
+
+export const usePrevious = (value: any) => {
+  const ref = useRef<typeof value>()
+
+  useEffect(() => {
+    ref.current = value
+  }, [value])
+
+  return ref.current
+}
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/common/components/CheckBoxInput.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/common/components/CheckBoxInput.tsx
@@ -1,18 +1,15 @@
-import React, { ChangeEvent, forwardRef } from 'react'
+import { ChangeEvent, forwardRef, MouseEvent } from 'react'

 type CheckBoxInputProps = {
  checked?: boolean
  disabled?: boolean
  testId?: string
  onChange?: (event: ChangeEvent<HTMLInputElement>) => void
+  onClick?: (event: MouseEvent<SVGElement>) => void
 }

 export const CheckBoxInput = forwardRef<HTMLInputElement, CheckBoxInputProps>(
-  ({ checked, disabled, testId, onChange }, ref) => {
-    function onCheckBoxButtonClick({ currentTarget }: React.MouseEvent<SVGElement>) {
-      !checked ? currentTarget.classList.add('explode') : currentTarget.classList.remove('explode')
-    }
-
+  ({ checked, disabled, testId, onChange, onClick }, ref) => {
    return (
      <label className="checkbox-container">
        <input
@@ -29,7 +26,7 @@ export const CheckBoxInput = forwardRef<HTMLInputElement, CheckBoxInputProps>(
          xmlnsXlink="http://www.w3.org/1999/xlink"
          viewBox="3 2 22 20"
          className="checkbox-button"
-          onClick={onCheckBoxButtonClick}
+          onClick={onClick}
        >
          <use xlinkHref="#checkbox-square" className="checkbox-square"></use>
          <use xlinkHref="#checkbox-mark" className="checkbox-mark"></use>
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/CreateTask.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/CreateTask.tsx
@@ -53,7 +53,8 @@ const CreateTask: React.FC<CreateTaskProps> = ({ group }) => {
  }

  useDebouncedCallback(() => {
-    if (group.draft !== undefined && taskDraft !== group.draft) {
+    const currentDraft = group.draft ?? ''
+    if (currentDraft !== taskDraft) {
      dispatch(tasksGroupDraft({ groupName, draft: taskDraft }))
    }
  })
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/InvalidContentError.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/InvalidContentError.tsx
@@ -13,7 +13,11 @@ const Container = styled.div`

 const InvalidContentError: React.FC = () => {
  const lastError = useAppSelector((state) => state.tasks.lastError)
-  return <Container>{lastError || 'An unknown error has occurred, and the content cannot be displayed.'}</Container>
+  if (!lastError) {
+    return <></>
+  }
+
+  return <Container>{lastError}</Container>
 }

 export default InvalidContentError
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskGroup.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskGroup.tsx
@@ -1,4 +1,3 @@
-import { useState } from 'react'
 import styled from 'styled-components'

 import { useAppDispatch, useAppSelector } from '../../app/hooks'
@@ -10,6 +9,7 @@ import TaskSectionList from './TaskSectionList'

 import TaskGroupOptions from './TaskGroupOptions'

+import { useEffect, useState } from 'react'
 import { CircularProgressBar, GenericInlineText, MainTitle, RoundButton } from '../../common/components'
 import { ChevronDownIcon, ChevronUpIcon, ReorderIcon } from '../../common/components/icons'

@@ -26,14 +26,6 @@ const TaskGroupContainer = styled.div<{ isLast?: boolean }>`
  }
 `

-type CollapsableContainerProps = {
-  collapsed: boolean
-}
-
-const CollapsableContainer = styled.div<CollapsableContainerProps>`
-  display: ${({ collapsed }) => (collapsed ? 'none' : 'block')};
-`
-
 type TaskGroupProps = {
  group: GroupModel
  isDragging: boolean
@@ -69,7 +61,6 @@ const TaskGroup: React.FC<TaskGroupProps> = ({
  const allTasksCompleted = totalTasks > 0 && totalTasks === completedTasks

  function handleCollapse() {
-    dispatch(tasksGroupCollapsed({ groupName, type: 'group', collapsed: !collapsed }))
    setCollapsed(!collapsed)
  }

@@ -80,6 +71,10 @@ const TaskGroup: React.FC<TaskGroupProps> = ({
    setCollapsed(false)
  }

+  useEffect(() => {
+    dispatch(tasksGroupCollapsed({ groupName, type: 'group', collapsed }))
+  }, [collapsed, dispatch, groupName])
+
  return (
    <TaskGroupContainer
      ref={innerRef}
@@ -119,10 +114,12 @@ const TaskGroup: React.FC<TaskGroupProps> = ({
        )}
      </div>

-      <CollapsableContainer collapsed={collapsed}>
-        <CreateTask group={group} />
-        <TaskSectionList group={group} />
-      </CollapsableContainer>
+      {!collapsed && (
+        <>
+          <CreateTask group={group} />
+          <TaskSectionList group={group} />
+        </>
+      )}
    </TaskGroupContainer>
  )
 }
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskItem.scss
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskItem.scss
@@ -7,7 +7,7 @@ $transition-duration: 750ms;
  }
  0% {
    opacity: 1;
-    max-height: 25px;
+    max-height: 100px;
  }
 }

@@ -18,18 +18,30 @@ $transition-duration: 750ms;
  }
  100% {
    opacity: 1;
-    max-height: 25px;
+    max-height: 100px;
  }
 }

-.task-item.fade-out {
+.fade-out {
  animation: fadeOut ease $transition-duration;
-  animation-delay: $transition-duration;
+  animation-delay: 0s;
  animation-fill-mode: forwards;
 }

-.task-item.fade-in {
+.fade-in {
  animation: fadeIn ease $transition-duration;
  animation-delay: 0s;
  animation-fill-mode: forwards;
 }
+
+.completed {
+  animation-delay: 1.8s;
+}
+
+.opened {
+  animation-delay: 1.2s;
+}
+
+.hide {
+  display: none;
+}
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskItem.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskItem.tsx
@@ -1,6 +1,6 @@
 import './TaskItem.scss'

-import { ChangeEvent, createRef, KeyboardEvent, useState } from 'react'
+import { ChangeEvent, KeyboardEvent, MouseEvent, useRef, useState } from 'react'
 import styled from 'styled-components'

 import { useAppDispatch, useAppSelector, useDebouncedCallback, useResize } from '../../app/hooks'
@@ -8,30 +8,11 @@ import { taskDeleted, TaskModel, taskModified, taskToggled } from './tasks-slice

 import { CheckBoxInput, TextAreaInput } from '../../common/components'

-/**
- * A delay in the dispatch function, when a task is opened.
- * Necessary to allow for transitions to occur.
- */
-const DISPATCH_OPENED_DELAY_MS = 1_250
-
-/**
- * A delay in the dispatch function, when a task is completed.
- * Necessary to allow for transitions to occur.
- */
-const DISPATCH_COMPLETED_DELAY_MS = 1_850
-
-const Container = styled.div<{ completed?: boolean }>`
+const Container = styled.div`
  align-content: center;
  align-items: center;
  display: flex;
  flex-direction: row;
-
-  ${({ completed }) =>
-    completed &&
-    `
-    color: var(--sn-stylekit-info-color);
-  `}
-
  min-width: 10%;
  max-width: 90%;
 `
@@ -39,11 +20,11 @@ const Container = styled.div<{ completed?: boolean }>`
 export type TaskItemProps = {
  task: TaskModel
  groupName: string
-  innerRef?: (element?: HTMLElement | null | undefined) => any
 }

-const TaskItem: React.FC<TaskItemProps> = ({ task, groupName, innerRef, ...props }) => {
-  const textAreaRef = createRef<HTMLTextAreaElement>()
+const TaskItem: React.FC<TaskItemProps> = ({ task, groupName }) => {
+  const containerRef = useRef<HTMLDivElement>(null)
+  const textAreaRef = useRef<HTMLTextAreaElement>(null)

  const dispatch = useAppDispatch()

@@ -68,12 +49,14 @@ const TaskItem: React.FC<TaskItemProps> = ({ task, groupName, innerRef, ...props
    const singleLineHeight = 20
    const currentHeight = parseFloat(textarea.style.height)

+    const containerElement = containerRef.current
+
    if (currentHeight > singleLineHeight) {
-      textarea.parentElement?.classList.add('align-baseline')
-      textarea.parentElement?.classList.remove('align-center')
+      containerElement?.classList.add('align-baseline')
+      containerElement?.classList.remove('align-center')
    } else {
-      textarea.parentElement?.classList.add('align-center')
-      textarea.parentElement?.classList.remove('align-baseline')
+      containerElement?.classList.add('align-center')
+      containerElement?.classList.remove('align-baseline')
    }
  }

@@ -81,15 +64,27 @@ const TaskItem: React.FC<TaskItemProps> = ({ task, groupName, innerRef, ...props
    const newCompletedState = !completed
    setCompleted(newCompletedState)

-    newCompletedState
-      ? textAreaRef.current!.classList.add('cross-out')
-      : textAreaRef.current!.classList.add('no-text-decoration')
+    const textarea = textAreaRef.current

-    const dispatchDelay = newCompletedState ? DISPATCH_COMPLETED_DELAY_MS : DISPATCH_OPENED_DELAY_MS
+    if (newCompletedState) {
+      textarea?.classList.add(...['cross-out', 'info-color'])
+    } else {
+      textarea?.classList.add('no-text-decoration')
+    }

-    setTimeout(() => {
-      dispatch(taskToggled({ id: task.id, groupName }))
-    }, dispatchDelay)
+    dispatch(taskToggled({ id: task.id, groupName }))
+  }
+
+  function onCheckBoxClick({ currentTarget }: MouseEvent<SVGElement>) {
+    const parentElement = containerRef.current?.parentElement
+
+    if (task.completed) {
+      currentTarget.classList.remove('explode')
+      parentElement?.classList.add('completed')
+    } else {
+      currentTarget.classList.add('explode')
+      parentElement?.classList.add('opened')
+    }
  }

  function onTextChange(event: ChangeEvent<HTMLTextAreaElement>) {
@@ -125,8 +120,14 @@ const TaskItem: React.FC<TaskItemProps> = ({ task, groupName, innerRef, ...props
  useResize(textAreaRef, resizeTextArea)

  return (
-    <Container data-testid="task-item" completed={completed} ref={innerRef} {...props}>
-      <CheckBoxInput testId="check-box-input" checked={completed} disabled={!canEdit} onChange={onCheckBoxToggle} />
+    <Container className="task-item" data-testid="task-item" ref={containerRef}>
+      <CheckBoxInput
+        testId="check-box-input"
+        checked={completed}
+        disabled={!canEdit}
+        onChange={onCheckBoxToggle}
+        onClick={onCheckBoxClick}
+      />
      <TextAreaInput
        testId="text-area-input"
        className="text-area-input"
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskSectionList.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TaskSectionList.tsx
@@ -51,7 +51,13 @@ const TaskSectionList: React.FC<TaskSectionListProps> = ({ group }) => {
        )
        return (
          <DragDropContext key={`${section.id}-section-dnd`} onDragEnd={onDragEnd}>
-            <TasksSection testId={`${section.id}-section`} groupName={group.name} section={section} tasks={tasks}>
+            <TasksSection
+              testId={`${section.id}-section`}
+              groupName={group.name}
+              section={section}
+              tasks={tasks}
+              allTasks={group.tasks}
+            >
              {section.id === 'completed-tasks' && tasks.length > 0 && <CompletedTasksActions groupName={group.name} />}
            </TasksSection>
          </DragDropContext>
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TasksSection.tsx
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/features/tasks/TasksSection.tsx
@@ -1,11 +1,11 @@
 import './TasksSection.scss'

-import React, { useState } from 'react'
+import React, { useEffect, useState } from 'react'
 import { Draggable, DraggingStyle, Droppable, NotDraggingStyle } from 'react-beautiful-dnd'
 import { CSSTransition, TransitionGroup } from 'react-transition-group'
 import styled from 'styled-components'

-import { useAppDispatch, useAppSelector } from '../../app/hooks'
+import { useAppDispatch, useAppSelector, usePrevious } from '../../app/hooks'
 import { RoundButton, SubTitle } from '../../common/components'
 import { SectionModel, TaskModel, tasksGroupCollapsed } from './tasks-slice'

@@ -21,8 +21,8 @@ const SectionHeader = styled.div`
  }
 `

-const InnerTasksContainer = styled.div<{ collapsed: boolean }>`
-  display: ${({ collapsed }) => (collapsed ? 'none' : 'flex')};
+const InnerTasksContainer = styled.div`
+  display: flex;
  flex-direction: column;

  & > *:not(:last-child) {
@@ -42,6 +42,21 @@ const Wrapper = styled.div`
  color: var(--sn-stylekit-foreground-color);
 `

+const COMPLETED_TASK_TIMEOUT_MS = 1_800
+const OPEN_TASK_TIMEOUT_MS = 1_200
+
+type TransitionTimeout = {
+  enter: number
+  exit: number
+}
+
+function getTimeout(completed?: boolean): TransitionTimeout {
+  return {
+    enter: completed ? COMPLETED_TASK_TIMEOUT_MS : OPEN_TASK_TIMEOUT_MS,
+    exit: !completed ? COMPLETED_TASK_TIMEOUT_MS : OPEN_TASK_TIMEOUT_MS,
+  }
+}
+
 const getItemStyle = (isDragging: boolean, draggableStyle?: DraggingStyle | NotDraggingStyle) => ({
  ...draggableStyle,
  ...(isDragging && {
@@ -54,21 +69,27 @@ type TasksSectionProps = {
  groupName: string
  tasks: TaskModel[]
  section: SectionModel
+  allTasks?: TaskModel[]
  testId?: string
 }

-const TasksSection: React.FC<TasksSectionProps> = ({ groupName, tasks, section, testId, children }) => {
+const TasksSection: React.FC<TasksSectionProps> = ({ groupName, tasks, section, allTasks, testId, children }) => {
  const dispatch = useAppDispatch()
  const canEdit = useAppSelector((state) => state.settings.canEdit)
  const droppableId = `${section.id}-droppable`

  const [collapsed, setCollapsed] = useState<boolean>(!!section.collapsed)

-  const handleCollapse = () => {
-    dispatch(tasksGroupCollapsed({ groupName, type: section.id, collapsed: !collapsed }))
+  function handleCollapse() {
    setCollapsed(!collapsed)
  }

+  const prevTasks: TaskModel[] = usePrevious(allTasks)
+
+  useEffect(() => {
+    dispatch(tasksGroupCollapsed({ groupName, type: section.id, collapsed }))
+  }, [collapsed, dispatch, groupName, section.id])
+
  return (
    <OuterContainer
      data-testid={testId}
@@ -87,80 +108,68 @@ const TasksSection: React.FC<TasksSectionProps> = ({ groupName, tasks, section,
                </RoundButton>
              )}
            </SectionHeader>
-            <InnerTasksContainer
-              {...provided.droppableProps}
-              className={`${section.id}-container`}
-              collapsed={collapsed}
-              ref={provided.innerRef}
-            >
-              <TransitionGroup component={null} childFactory={(child) => React.cloneElement(child)}>
-                {tasks.map((task, index) => (
-                  <CSSTransition
-                    key={`${task.id}-${!!task.completed}`}
-                    classNames={{
-                      enter: 'fade-in',
-                      enterActive: 'fade-in',
-                      enterDone: 'fade-in',
-                      exit: 'fade-out',
-                      exitActive: 'fade-out',
-                      exitDone: 'fade-out',
-                    }}
-                    timeout={{
-                      enter: 1_500,
-                      exit: 1_250,
-                    }}
-                    onEnter={(node: HTMLElement) => {
-                      node.classList.remove('explode')
-                    }}
-                    onEntered={(node: HTMLElement) => {
-                      node.classList.remove('fade-in')
+            {!collapsed && (
+              <InnerTasksContainer
+                {...provided.droppableProps}
+                className={`${section.id}-container`}
+                ref={provided.innerRef}
+              >
+                <TransitionGroup component={null}>
+                  {tasks.map((task, index) => {
+                    const timeout = getTimeout(task.completed)
+                    return (
+                      <CSSTransition
+                        key={task.id}
+                        timeout={timeout}
+                        classNames={{
+                          enter: 'fade-in',
+                          enterActive: 'fade-in',
+                          enterDone: 'fade-in',
+                          exit: 'fade-out',
+                          exitActive: 'fade-out',
+                          exitDone: 'fade-out',
+                        }}
+                        onEnter={(node: HTMLElement) => {
+                          const exists = prevTasks.find((t) => t.id === task.id)
+                          exists && node.classList.add('hide')

-                      const completed = !!task.completed
-                      completed && node.classList.add('explode')
+                          const completed = !!task.completed
+                          completed && node.classList.add('explode')
+                          !completed && node.classList.remove('explode')
+                        }}
+                        onEntered={(node: HTMLElement) => {
+                          node.classList.remove('hide')
+                          setTimeout(() => node.classList.remove(...['fade-in', 'explode']), timeout.enter)
+                        }}
+                      >
+                        <Draggable
+                          key={`draggable-${task.id}`}
+                          draggableId={`draggable-${task.id}`}
+                          index={index}
+                          isDragDisabled={!canEdit}
+                        >
+                          {({ innerRef, draggableProps, dragHandleProps }, { isDragging }) => {
+                            const { style, ...restDraggableProps } = draggableProps
+                            return (
+                              <div
+                                style={getItemStyle(isDragging, style)}
+                                ref={innerRef}
+                                {...restDraggableProps}
+                                {...dragHandleProps}
+                              >
+                                <TaskItem key={`task-item-${task.id}`} task={task} groupName={groupName} />
+                              </div>
+                            )
+                          }}
+                        </Draggable>
+                      </CSSTransition>
+                    )
+                  })}
+                </TransitionGroup>

-                      node.addEventListener(
-                        'animationend',
-                        () => {
-                          node.classList.remove('explode')
-                        },
-                        false,
-                      )
-                    }}
-                    onExited={(node: HTMLElement) => {
-                      node.classList.remove('fade-out')
-                    }}
-                    addEndListener={(node, done) => {
-                      done()
-                    }}
-                    mountOnEnter
-                    unmountOnExit
-                  >
-                    <Draggable
-                      key={`draggable-${task.id}`}
-                      draggableId={`draggable-${task.id}`}
-                      index={index}
-                      isDragDisabled={!canEdit}
-                    >
-                      {({ innerRef, draggableProps, dragHandleProps }, { isDragging }) => {
-                        const { style, ...restDraggableProps } = draggableProps
-                        return (
-                          <div className="task-item" style={getItemStyle(isDragging, style)} {...restDraggableProps}>
-                            <TaskItem
-                              key={`task-item-${task.id}`}
-                              task={task}
-                              groupName={groupName}
-                              innerRef={innerRef}
-                              {...dragHandleProps}
-                            />
-                          </div>
-                        )
-                      }}
-                    </Draggable>
-                  </CSSTransition>
-                ))}
-              </TransitionGroup>
-              {provided.placeholder}
-            </InnerTasksContainer>
+                {provided.placeholder}
+              </InnerTasksContainer>
+            )}
            <ChildrenContainer addMargin={section.id === 'completed-tasks'} items={tasks.length}>
              {children}
            </ChildrenContainer>
--- a/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/stylesheets/main.scss
+++ b/packages/components/src/Packages/Editors/org.standardnotes.advanced-checklist/src/stylesheets/main.scss
@@ -49,6 +49,10 @@ html {
    fill: var(--sn-stylekit-info-color);
  }

+  .info-color {
+    color: var(--sn-stylekit-info-color);
+  }
+
  .sn-icon-button {
    border-width: 0;