Merge branch 'lib-classifier_survey-task-design-changes' into lib-cla…

…ssifier_chooser-styling-updates
zooniverse · Dec 17, 2024 · af889d3 · af889d3
2 parents fb25f35 + a111a79
commit af889d3
Show file tree

Hide file tree

Showing 10 changed files with 206 additions and 118 deletions.
diff --git a/packages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.js b/packages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.js
@@ -28,6 +28,13 @@ export function SurveyTask ({
     setAnswers(answers)
   }
 
+  function handleCancel (selectedChoice) {
+    setAnswers({})
+    setPreviousChoice(selectedChoice)
+    setSelectedChoice('')
+    annotation.setChoiceInProgress(false)
+  }
+
   function handleChoice (selectedChoice) {
     if (selectedChoice === '') {
       annotation.setChoiceInProgress(false)
@@ -94,8 +101,8 @@ export function SurveyTask ({
             answers={answers}
             choiceId={selectedChoice}
             handleAnswers={handleAnswers}
+            handleCancel={handleCancel}
             handleChoice={handleChoice}
-            handleDelete={handleDelete}
             onIdentify={handleIdentify}
             task={task}
           />

diff --git a/packages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.userClicks.spec.js b/packages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.userClicks.spec.js
@@ -173,9 +173,9 @@ describe('SurveyTask with user clicks', function () {
       await user.click(choiceButton)
     })
 
-    it('should show the choice description', async function () {
-      const choiceDescription = screen.getByText('It\'s a fire. Pretty sure you know what this looks like.')
-      expect(choiceDescription).to.be.ok()
+    it('should show the "More info" button', async function () {
+      const choiceMoreInfoButton = screen.getByRole('button', { name: 'SurveyTask.Choice.moreInfo' })
+      expect(choiceMoreInfoButton).to.be.ok()
     })
 
     it('should show choice images', async function () {
@@ -184,12 +184,12 @@ describe('SurveyTask with user clicks', function () {
     })
 
     it('should show choices with closed choice focused when Not This button is clicked', async function () {
-      const notThisButton = screen.getByText('SurveyTask.Choice.notThis')
+      const cancelButton = screen.getByText('SurveyTask.Choice.cancel')
       // close choice (Fire) component
-      await user.click(notThisButton)
-      // confirm choice (Fire) description, and therefore choice, is not shown
-      const choiceDescription = screen.queryByText('It\'s a fire. Pretty sure you know what this looks like.')
-      expect(choiceDescription).to.be.null()
+      await user.click(cancelButton)
+      // confirm choice "More info" button is not shown, therefore choice is closed
+      const choiceMoreInfoButton = screen.queryByRole('button', { name: 'SurveyTask.Choice.moreInfo' })
+      expect(choiceMoreInfoButton).to.be.null()
       // confirm choices are shown
       const choiceButtons = document.querySelector('[role=menu]').querySelectorAll('[role=menuitem]')
       expect(choiceButtons.length).to.equal(6)
@@ -202,9 +202,9 @@ describe('SurveyTask with user clicks', function () {
       const identifyButton = screen.getByTestId('choice-identify-button')
       // identify choice (Fire) and close choice (Fire) component
       await user.click(identifyButton)
-      // confirm choice (Fire) description, and therefore choice, is not shown
-      const choiceDescription = screen.queryByText('It\'s a fire. Pretty sure you know what this looks like.')
-      expect(choiceDescription).to.be.null()
+      // confirm choice "More info" button is not shown, therefore choice is closed
+      const choiceMoreInfoButton = screen.queryByRole('button', { name: 'SurveyTask.Choice.moreInfo' })
+      expect(choiceMoreInfoButton).to.be.null()
       // confirm choices are shown
       const choiceButtons = document.querySelector('[role=menu]').querySelectorAll('[role=menuitem]')
       expect(choiceButtons.length).to.equal(6)

diff --git a/...ages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.userKeystrokes.spec.js b/...ages/lib-classifier/src/plugins/tasks/survey/components/SurveyTask.userKeystrokes.spec.js
@@ -166,9 +166,9 @@ describe('SurveyTask with user keystrokes', function () {
       await user.keyboard('[Enter]')
     })
 
-    it('should show the choice description', async function () {
-      const choiceDescription = screen.getByText('Not as awesome as a pangolin, but surprisingly big.')
-      expect(choiceDescription).to.be.ok()
+    it('should show the "more info" button', async function () {
+      const choiceMoreInfoButton = screen.getByRole('button', { name: 'SurveyTask.Choice.moreInfo' })
+      expect(choiceMoreInfoButton).to.be.ok()
     })
 
     it('should show choice images', async function () {

diff --git a/packages/lib-classifier/src/plugins/tasks/survey/components/components/Choice/Choice.js b/packages/lib-classifier/src/plugins/tasks/survey/components/components/Choice/Choice.js
@@ -1,6 +1,7 @@
-import { Box, Button, Carousel, Heading, Paragraph } from 'grommet'
+import { Box, Button, Carousel, Collapsible, Heading, Paragraph, Text } from 'grommet'
+import { FormDown, FormUp } from 'grommet-icons'
 import PropTypes from 'prop-types'
-import { useEffect, useRef } from 'react'
+import { useEffect, useRef, useState } from 'react'
 import styled from 'styled-components'
 import { useTranslation } from '@translations/i18n'
 import { PrimaryButton, Media } from '@zooniverse/react-components'
@@ -18,15 +19,31 @@ const StyledBox = styled(Box)`
   }
 `
 
+const StyledButton = styled(Button)`
+  border: 1px solid ${props => props.theme.global.colors.brand};
+  border-radius: 4px;
+  flex: 1 0;
+  margin-right: 1ch;
+  max-width: 350px;
+`
+
+const StyledPrimaryButton = styled(PrimaryButton)`
+  border-radius: 4px;
+  flex: 1 0;
+`
+
+const DEFAULT_HANDLER = () => true
+
 function Choice({
   answers = {},
   choiceId = '',
-  handleAnswers = () => {},
-  handleChoice = () => {},
-  handleDelete = () => {},
-  onIdentify = () => {},
+  handleAnswers = DEFAULT_HANDLER,
+  handleCancel = DEFAULT_HANDLER,
+  handleChoice = DEFAULT_HANDLER,
+  onIdentify = DEFAULT_HANDLER,
   task
 }) {
+  const [showInfo, setShowInfo] = useState(false)
   const {
     choices,
     images,
@@ -46,6 +63,25 @@ function Choice({
   const choice = choices?.get(choiceId) || {}
   const questionIds = getQuestionIds(choiceId, task)
   const allowIdentify = allowIdentification(answers, choiceId, task)
+  const InfoLabel = (
+    <Box
+      align='center'
+      direction='row'
+      gap='xsmall'
+    >
+      {showInfo ? (
+        <>
+          <Text>{t('SurveyTask.Choice.lessInfo')}</Text>
+          <FormUp aria-hidden="true" />
+        </>
+      ) : (
+        <>
+          <Text>{t('SurveyTask.Choice.moreInfo')}</Text>
+          <FormDown aria-hidden="true" />
+        </>
+      )}
+    </Box>
+  )
 
   function handleKeyDown (event) {
     if (event.key === 'Escape') {
@@ -58,23 +94,24 @@ function Choice({
       ref={choiceRef}
       aria-labelledby='choice-label'
       background={{
-        dark: 'dark-1',
-        light: 'light-1'
+        dark: 'dark-3',
+        light: 'neutral-6'
       }}
-      elevation='large'
       flex='grow'
       forwardedAs='section'
       onKeyDown={handleKeyDown}
-      pad='small'
       tabIndex={0}
     >
       {choice.images?.length > 0 && (
         <Carousel
           alignSelf='center'
           controls='arrows'
           data-testid='choice-images'
-          height={{ max: 'medium' }}
-          width={{ max: 'medium' }}
+          height={{
+            max: 'medium',
+            min: '160px'
+          }}
+          width='100%'
         >
           {choice.images.map((filename, index) => (
             <Media
@@ -85,57 +122,90 @@ function Choice({
           ))}
         </Carousel>
       )}
-      <Heading
-        id='choice-label'
-      >
-        {strings.get(`choices.${choiceId}.label`)}
-      </Heading>
-      <Paragraph>{strings.get(`choices.${choiceId}.description`)}</Paragraph>
-      {choice.confusionsOrder?.length > 0 && (
-        <ConfusedWith
-          choices={choices}
-          choiceId={choiceId}
-          confusions={choice.confusions}
-          confusionsOrder={choice.confusionsOrder}
-          handleChoice={handleChoice}
-          images={images}
-          strings={strings}
-        />
-      )}
-      {questionIds.length > 0 && (
-        <Questions
-          answers={answers}
-          questionIds={questionIds}
-          questions={questions}
-          setAnswers={handleAnswers}
-          strings={strings}
-        />
-      )}
       <Box
-        border={{
-          color: 'light-5',
-          side: 'top',
-          size: 'small'
+        pad={{
+          bottom: '10px',
+          top: '20px'
         }}
-        direction='row'
-        fill='horizontal'
-        gap='xsmall'
-        justify='center'
-        margin={{ top: 'small' }}
-        pad={{ top: 'small' }}
       >
-        <Button
+        <Box
+          direction='row'
           fill='horizontal'
-          label={t('SurveyTask.Choice.notThis')}
-          onClick={() => handleDelete(choiceId)}
-        />
-        <PrimaryButton
-          data-testid='choice-identify-button'
-          disabled={!allowIdentify}
+          height='auto'
+          justify='between'
+          margin={{ bottom: '20px' }}
+        >
+          <Heading
+            id='choice-label'
+            color={{
+              dark: 'accent-1',
+              light: 'neutral-7'
+            }}
+            margin='none'
+            size='1.5rem'
+            weight='bold'
+          >
+            {strings.get(`choices.${choiceId}.label`)}
+          </Heading>
+          <Button
+            label={InfoLabel}
+            onClick={() => setShowInfo(!showInfo)}          
+            plain
+          />
+        </Box>
+        <Collapsible
+          open={showInfo}
+        >
+          <Box margin={{ bottom: '20px' }}>
+            <Paragraph margin='none'>
+              {strings.get(`choices.${choiceId}.description`)}
+            </Paragraph>
+            {choice.confusionsOrder?.length > 0 && (
+              <ConfusedWith
+                choices={choices}
+                choiceId={choiceId}
+                confusions={choice.confusions}
+                confusionsOrder={choice.confusionsOrder}
+                handleChoice={handleChoice}
+                images={images}
+                strings={strings}
+              />
+            )}
+          </Box>
+        </Collapsible>
+        {questionIds.length > 0 && (
+          <Questions
+            answers={answers}
+            questionIds={questionIds}
+            questions={questions}
+            setAnswers={handleAnswers}
+            strings={strings}
+          />
+        )}
+        <Box
+          border={{
+            color: 'light-5',
+            side: 'bottom',
+            size: 'small'
+          }}
+          direction='row'
           fill='horizontal'
-          label={t('SurveyTask.Choice.identify')}
-          onClick={() => onIdentify()}
-        />
+          justify='center'
+          pad={{ vertical: '20px' }}
+        >
+          <StyledButton
+            fill='horizontal'
+            label={t('SurveyTask.Choice.cancel')}
+            onClick={() => handleCancel(choiceId)}
+          />
+          <StyledPrimaryButton
+            data-testid='choice-identify-button'
+            disabled={!allowIdentify}
+            fill='horizontal'
+            label={t('SurveyTask.Choice.identify')}
+            onClick={() => onIdentify()}
+          />
+        </Box>
       </Box>
     </StyledBox>
   )
@@ -150,8 +220,8 @@ Choice.propTypes = {
   ),
   choiceId: PropTypes.string,
   handleAnswers: PropTypes.func,
+  handleCancel: PropTypes.func,
   handleChoice: PropTypes.func,
-  handleDelete: PropTypes.func,
   onIdentify: PropTypes.func,
   task: PropTypes.shape({
     help: PropTypes.string,