add enhanced rubrics readonly input changes

this commit modifies the read only view of the rubrics to remove the
text inputs for comments and display as plain text. this change also
updates the UI from the latest design for the comment section. removing
the "Comment" link and displaying the comment input/text directly for
each criteria. this commit also makes some style changes to the "preview
mode" when displaying the rubrics in the Rubric Builder and List pages.

closes EVAL-4173
flag=enhanced_rubrics

setup:
- have a peer reviewed assignment
- have a rubric attached to the assignment
- assign and have both students submit an assignment

test plan:
- from both the Rubric Builder and List pages, view the rubric in
  preview mode. Verify that the rubric is in the "instructor view" but
  does not display the Submit Assessment button.
- navigate to Speedgrader for the assignment and open the rubric.
- verify the UI changes of the comment for the rubric. Verify the
  "Clear" button clears out any comment text. Save the rubric and verify
  the comment is saved.
- navigate as a student to the assignments page for the assignment with
  the rubric. Verify the rubric is in the "preview mode" and the comment
  is displayed as plain text.
- as the student, navigate to the peer reviewed assignment. verify that
  you can assess the rubric with some comments. reload the page and
  verify that the rubric is in "read only" mode and comments are
  displayed as plain text.

Change-Id: I032329a70895b6d6a61d6dd7721ecfd9bdf55603
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/347920
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Derek Williams <derek.williams@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
Product-Review: Melissa Kruger <melissa.kruger@instructure.com>

ui/features/assignments_show_student/react/components/__tests__/RubricTab.test.jsx

@@ -373,14 +373,6 @@ describe('RubricTab', () => {
         window.ENV.FEATURES.enhanced_rubrics = false
       })
 
-      it('displays comments', async () => {
-        const {getByTestId} = render(<RubricTab {...props} />)
-        const assessmentData = props.assessments[0].data[0]
-        const commentSection = getByTestId(`comment-text-area-${assessmentData.criterion_id}`)
-        expect(commentSection).toHaveTextContent(assessmentData.comments)
-        expect(commentSection).toBeDisabled()
-      })
-
       it('displays the name of the assessor if present', async () => {
         const {findByLabelText, findByText} = render(<RubricTab {...props} />)
         fireEvent.click(await findByLabelText('Select Grader'))
@@ -391,8 +383,8 @@ describe('RubricTab', () => {
         const {getByTestId} = render(<RubricTab {...props} />)
         const assessmentData = props.assessments[0].data[0]
         const {criterion_id} = assessmentData
-        const commentSection = getByTestId(`comment-text-area-${criterion_id}`)
-        expect(commentSection).toBeDisabled()
+        const commentSection = getByTestId('comment-preview-text-area')
+        expect(commentSection).toHaveTextContent(assessmentData.comments)
         const ratingSection = getByTestId(`traditional-criterion-${criterion_id}-ratings-0`)
         expect(ratingSection).toBeDisabled()
       })

ui/features/rubrics/components/RubricForm/index.tsx

@@ -510,7 +510,7 @@ export const RubricForm = ({rootOutcomeGroup}: RubricFormComponentProp) => {
       />
       <RubricAssessmentTray
         isOpen={isPreviewTrayOpen}
-        isPreviewMode={true}
+        isPreviewMode={false}
         rubric={rubricForm}
         rubricAssessmentData={[]}
         onDismiss={() => setIsPreviewTrayOpen(false)}

ui/features/rubrics/components/ViewRubrics/index.tsx

@@ -298,10 +298,13 @@ export const ViewRubrics = () => {
       <RubricAssessmentTray
         isLoading={isLoadingPreview}
         isOpen={isPreviewTrayOpen}
-        isPreviewMode={true}
+        isPreviewMode={false}
         rubric={rubricPreview}
         rubricAssessmentData={[]}
-        onDismiss={() => setIsPreviewTrayOpen(false)}
+        onDismiss={() => {
+          setRubricIdForPreview(undefined)
+          setIsPreviewTrayOpen(false)
+        }}
       />
 
       <UsedLocationsModal

ui/shared/rubrics/react/RubricAssessment/CriteriaReadonlyComment.tsx

@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2024 - present Instructure, Inc.
+ *
+ * This file is part of Canvas.
+ *
+ * Canvas is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Affero General Public License as published by the Free
+ * Software Foundation, version 3 of the License.
+ *
+ * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU Affero General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import React from 'react'
+import {useScope as useI18nScope} from '@canvas/i18n'
+import {View} from '@instructure/ui-view'
+import {Text} from '@instructure/ui-text'
+
+const I18n = useI18nScope('rubrics-assessment-tray')
+
+type CriteriaReadonlyCommentProps = {
+  commentText?: string
+}
+export const CriteriaReadonlyComment = ({commentText}: CriteriaReadonlyCommentProps) => {
+  return (
+    <View as="div" margin="0">
+      {commentText && (
+        <View as="div" margin="0">
+          <Text weight="bold">{I18n.t('Comment')}</Text>
+        </View>
+      )}
+      <Text data-testid="comment-preview-text-area">{commentText}</Text>
+    </View>
+  )
+}

ui/shared/rubrics/react/RubricAssessment/ModernView.tsx

@@ -29,6 +29,7 @@ import type {RubricAssessmentData, RubricCriterion, UpdateAssessmentData} from '
 import {TextArea} from '@instructure/ui-text-area'
 import {Checkbox} from '@instructure/ui-checkbox'
 import {CommentLibrary} from './CommentLibrary'
+import {CriteriaReadonlyComment} from './CriteriaReadonlyComment'
 
 const I18n = useI18nScope('rubrics-assessment-tray')
 
@@ -59,7 +60,7 @@ export const ModernView = ({
   onUpdateAssessmentData,
 }: ModernViewProps) => {
   return (
-    <View as="div" margin="0">
+    <View as="div" margin="0" overflowX="hidden">
       {criteria.map((criterion, index) => {
         const criterionAssessment = rubricAssessmentData.find(
           data => data.criterionId === criterion.id
@@ -282,20 +283,19 @@ export const CriterionRow = ({
           </Flex>
         ) : (
           <Flex direction="column">
-            <Flex.Item>
-              <Text weight="bold">{I18n.t('Comment')}</Text>
-            </Flex.Item>
             <Flex.Item margin="x-small 0 0 0" shouldGrow={true}>
-              <TextArea
-                label={<ScreenReaderContent>{I18n.t('Criterion Comment')}</ScreenReaderContent>}
-                readOnly={isPreviewMode}
-                data-testid={`modern-comment-area-${criterion.id}`}
-                width="100%"
-                height="38px"
-                value={commentText}
-                onChange={e => setCommentText(e.target.value)}
-                onBlur={e => updateAssessmentData({comments: e.target.value})}
-              />
+              {isPreviewMode ? (
+                <CriteriaReadonlyComment commentText={commentText} />
+              ) : (
+                <TextArea
+                  label={I18n.t('Comment')}
+                  size="small"
+                  value={commentText}
+                  onChange={e => setCommentText(e.target.value)}
+                  onBlur={() => updateAssessmentData({comments: commentText})}
+                  placeholder={I18n.t('Leave a comment')}
+                />
+              )}
             </Flex.Item>
           </Flex>
         )}

ui/shared/rubrics/react/RubricAssessment/RubricAssessmentContainer.tsx

@@ -177,7 +177,7 @@ export const RubricAssessmentContainer = ({
             {renderViewContainer()}
           </View>
         </Flex.Item>
-        {!isPreviewMode && (
+        {!isPreviewMode && onSubmit && (
           <Flex.Item as="footer">
             <AssessmentFooter onSubmit={onSubmit} />
           </Flex.Item>

ui/shared/rubrics/react/RubricAssessment/RubricAssessmentTray.tsx

@@ -146,7 +146,7 @@ export const RubricAssessmentTray = ({
           selectedViewMode={viewMode}
           onAccessorChange={onAccessorChange}
           onDismiss={onDismiss}
-          onSubmit={() => onSubmit?.(rubricAssessmentDraftData)}
+          onSubmit={onSubmit ? () => onSubmit?.(rubricAssessmentDraftData) : undefined}
           onViewModeChange={mode => setViewMode(mode)}
           onUpdateAssessmentData={onUpdateAssessmentData}
         />

ui/shared/rubrics/react/RubricAssessment/TraditionalView.tsx

@@ -22,16 +22,16 @@ import {colors} from '@instructure/canvas-theme'
 import {View} from '@instructure/ui-view'
 import {Flex} from '@instructure/ui-flex'
 import {Text} from '@instructure/ui-text'
-import {IconChatLine} from '@instructure/ui-icons'
 import {ScreenReaderContent} from '@instructure/ui-a11y-content'
 import {TextArea} from '@instructure/ui-text-area'
-import {Link} from '@instructure/ui-link'
 import {possibleString} from '../Points'
 import type {RubricAssessmentData, RubricCriterion, UpdateAssessmentData} from '../types/rubric'
 import {Grid} from '@instructure/ui-grid'
 import {TextInput} from '@instructure/ui-text-input'
 import {Checkbox} from '@instructure/ui-checkbox'
 import {CommentLibrary} from './CommentLibrary'
+import {CriteriaReadonlyComment} from './CriteriaReadonlyComment'
+import {Button} from '@instructure/ui-buttons'
 
 const I18n = useI18nScope('rubrics-assessment-tray')
 const {licorice} = colors
@@ -165,7 +165,6 @@ const CriterionRow = ({
   rubricSavedComments,
 }: CriterionRowProps) => {
   const [hoveredRatingIndex, setHoveredRatingIndex] = useState<number>()
-  const [isCommentsOpen, setIsCommentsOpen] = useState(hidePoints)
   const [commentText, setCommentText] = useState<string>(criterionAssessment?.comments ?? '')
   const [isSaveCommentChecked, setIsSaveCommentChecked] = useState(false)
 
@@ -183,13 +182,28 @@ const CriterionRow = ({
   }
 
   useEffect(() => {
-    if (criterionAssessment?.comments?.length && !isFreeFormCriterionComments) {
-      setIsCommentsOpen(true)
-    }
-
     setCommentText(criterionAssessment?.comments ?? '')
   }, [criterionAssessment, isFreeFormCriterionComments])
 
+  const setPoints = (value: string) => {
+    const points = Number(value)
+
+    if (!value.trim().length || Number.isNaN(points)) {
+      updateAssessmentData({points: undefined})
+      return
+    }
+
+    const selectedRating = criterion.ratings.find(rating => rating.points === points)
+
+    updateAssessmentData({
+      points,
+      description: selectedRating?.description,
+    })
+  }
+
+  const hideComments =
+    isFreeFormCriterionComments || (isPreviewMode && !criterionAssessment?.comments?.length)
+
   return (
     <View as="div" maxWidth="100%">
       <Flex>
@@ -374,7 +388,6 @@ const CriterionRow = ({
                             </Flex.Item>
                           </Flex>
                         </View>
-                        {/* </Flex.Item> */}
                       </Grid.Col>
                     )
                   })}
@@ -392,88 +405,67 @@ const CriterionRow = ({
               height="13.75rem"
               overflowY="auto"
             >
-              {isFreeFormCriterionComments ? (
-                <Flex direction="column" height="100%">
-                  <div style={{display: 'flex', alignItems: 'center'}}>
-                    <Flex.Item margin="small 0 0 0">
-                      <TextInput
-                        renderLabel={
-                          <ScreenReaderContent>{I18n.t('Criterion Score')}</ScreenReaderContent>
-                        }
-                        readOnly={isPreviewMode}
-                        data-testid={`comment-score-${criterion.id}`}
-                        placeholder="--"
-                        width="2.688rem"
-                        height="2.375rem"
-                        value={criterionAssessment?.points?.toString() || ''}
-                        onChange={e => {
-                          e.target.value
-                            ? updateAssessmentData({points: Number(e.target.value)})
-                            : updateAssessmentData({points: undefined})
-                        }}
-                        onBlur={e => {
-                          e.target.value
-                            ? updateAssessmentData({points: Number(e.target.value)})
-                            : updateAssessmentData({points: undefined})
-                        }}
-                      />
-                    </Flex.Item>
-                    <Flex.Item margin="small 0 0 x-small">
-                      <Text>{'/' + possibleString(criterion.points)}</Text>
-                    </Flex.Item>
-                  </div>
-                </Flex>
-              ) : (
-                <Flex direction="column" height="100%">
-                  <Flex.Item shouldGrow={true}>
-                    <Text>{!hidePoints && possibleString(criterion.points)}</Text>
+              <Flex direction="column" height="100%">
+                <div style={{display: 'flex', alignItems: 'center'}}>
+                  <Flex.Item margin="small 0 0 0">
+                    <TextInput
+                      renderLabel={
+                        <ScreenReaderContent>{I18n.t('Criterion Score')}</ScreenReaderContent>
+                      }
+                      readOnly={isPreviewMode}
+                      data-testid={`comment-score-${criterion.id}`}
+                      placeholder="--"
+                      width="2.688rem"
+                      height="2.375rem"
+                      value={criterionAssessment?.points?.toString() || ''}
+                      onChange={e => setPoints(e.target.value)}
+                      onBlur={e => setPoints(e.target.value)}
+                    />
                   </Flex.Item>
-                  <Flex.Item>
-                    <View as="div" position="relative" padding="0 0 x-small 0">
-                      <Link
-                        forceButtonRole={true}
-                        isWithinText={false}
-                        disabled={(criterionAssessment?.comments?.length ?? 0) > 0}
-                        data-testid={`rubric-comment-button-${criterion.id}`}
-                        onClick={() => setIsCommentsOpen(!isCommentsOpen)}
-                      >
-                        <IconChatLine />
-                        <View as="span" margin="0 0 0 xx-small">
-                          <Text size="small">Comment</Text>
-                        </View>
-                      </Link>
-                    </View>
+                  <Flex.Item margin="small 0 0 x-small">
+                    <Text>{'/' + possibleString(criterion.points)}</Text>
                   </Flex.Item>
-                </Flex>
-              )}
+                </div>
+              </Flex>
             </View>
           </Flex.Item>
         )}
       </Flex>
-      {isCommentsOpen && !isFreeFormCriterionComments && (
+      {!hideComments && (
         <View
           as="div"
-          padding="small medium"
+          padding="small"
           width="100%"
           borderWidth="0 small small small"
           themeOverride={{paddingMedium: '1.125rem'}}
         >
-          <Flex>
-            <Flex.Item margin="0 small 0 0">
-              <IconChatLine size="small" themeOverride={{sizeSmall: '1.125rem'}} />
-            </Flex.Item>
-            <Flex.Item shouldGrow={true}>
-              <TextArea
-                label={<ScreenReaderContent>{I18n.t('Criterion Comment')}</ScreenReaderContent>}
-                placeholder={I18n.t('Comment')}
-                readOnly={isPreviewMode}
-                data-testid={`comment-text-area-${criterion.id}`}
-                width="100%"
-                value={commentText}
-                onChange={e => setCommentText(e.target.value)}
-                onBlur={e => updateAssessmentData({comments: e.target.value})}
-              />
-            </Flex.Item>
+          <Flex direction="row-reverse">
+            {isPreviewMode ? (
+              <Flex.Item shouldGrow={true}>
+                <CriteriaReadonlyComment commentText={commentText} />
+              </Flex.Item>
+            ) : (
+              <>
+                <Flex.Item>
+                  <View margin="0 0 0 small" themeOverride={{marginSmall: '1rem'}}>
+                    <Button color="secondary" onClick={() => setCommentText('')}>
+                      {I18n.t('Clear')}
+                    </Button>
+                  </View>
+                </Flex.Item>
+                <Flex.Item shouldGrow={true}>
+                  <TextArea
+                    label={I18n.t('Comment')}
+                    placeholder={I18n.t('Leave a comment')}
+                    data-testid={`comment-text-area-${criterion.id}`}
+                    width="100%"
+                    value={commentText}
+                    onChange={e => setCommentText(e.target.value)}
+                    onBlur={e => updateAssessmentData({comments: e.target.value})}
+                  />
+                </Flex.Item>
+              </>
+            )}
           </Flex>
         </View>
       )}

ui/shared/rubrics/react/RubricAssessment/__tests__/RubricAssessmentTray.test.tsx

@@ -30,6 +30,7 @@ describe('RubricAssessmentTray Tests', () => {
         rubric={RUBRIC_DATA}
         rubricAssessmentData={[]}
         onDismiss={jest.fn()}
+        onSubmit={jest.fn()}
         {...props}
       />
     )
@@ -159,54 +160,9 @@ describe('RubricAssessmentTray Tests', () => {
       expect(getByTestId('traditional-criterion-2-ratings-0-selected')).toBeInTheDocument()
     })
 
-    it('should open the comments tray when the comments icon is clicked', () => {
-      const {getByTestId, queryByTestId} = renderComponent()
-      const commentsIcon = getByTestId('rubric-comment-button-1')
-      expect(queryByTestId('comment-text-area-1')).toBeNull()
-
-      fireEvent.click(commentsIcon)
-
-      expect(getByTestId('comment-text-area-1')).toBeInTheDocument()
-    })
-
-    it('should close the comments tray when the comments icon is clicked again', () => {
-      const {getByTestId, queryByTestId} = renderComponent()
-      const commentsIcon = getByTestId('rubric-comment-button-1')
-      fireEvent.click(commentsIcon)
-
-      expect(getByTestId('comment-text-area-1')).toBeInTheDocument()
-
-      fireEvent.click(commentsIcon)
-
-      expect(queryByTestId('comment-text-area-1')).toBeNull()
-    })
-
-    it('should disable the toggle comment button when there is existing comments for criteria', () => {
-      const rubricAssessmentData = [
-        {
-          criterionId: '1',
-          points: 4,
-          comments: 'existing comments',
-          id: '1',
-          commentsEnabled: true,
-          description: 'description',
-        },
-      ]
-      const {getByTestId} = renderComponent({rubricAssessmentData})
-      const commentsIcon = getByTestId('rubric-comment-button-1')
-      expect(commentsIcon).toBeDisabled()
-    })
-
-    it('should disable the toggle comment button after comments are added and blurred', () => {
+    it('should display the comments section', () => {
       const {getByTestId} = renderComponent()
-      const commentsIcon = getByTestId('rubric-comment-button-1')
-      fireEvent.click(commentsIcon)
-
-      const textArea = getByTestId('comment-text-area-1') as HTMLTextAreaElement
-      fireEvent.change(textArea, {target: {value: 'new comments'}})
-      fireEvent.blur(textArea)
-
-      expect(commentsIcon).toBeDisabled()
+      expect(getByTestId('comment-text-area-1')).toBeInTheDocument()
     })
 
     describe('Free Form Comments', () => {