From 6969bced64170876bf0468acac782395582eebeb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 12 Sep 2024 14:14:42 +0200 Subject: [PATCH] Add popover for grades in auto-grade assignments (#6677) --- .../dashboard/AssignmentActivity.tsx | 9 +- .../components/dashboard/GradeIndicator.tsx | 129 ++++++++++++++ .../components/dashboard/GradeStatusChip.tsx | 21 ++- .../dashboard/test/AssignmentActivity-test.js | 4 +- .../dashboard/test/GradeIndicator-test.js | 167 ++++++++++++++++++ 5 files changed, 318 insertions(+), 12 deletions(-) create mode 100644 lms/static/scripts/frontend_apps/components/dashboard/GradeIndicator.tsx create mode 100644 lms/static/scripts/frontend_apps/components/dashboard/test/GradeIndicator-test.js diff --git a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx index 6bc7f43878..9abaa7ac72 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx @@ -15,7 +15,7 @@ import { replaceURLParams } from '../../utils/url'; import DashboardActivityFilters from './DashboardActivityFilters'; import DashboardBreadcrumbs from './DashboardBreadcrumbs'; import FormattedDate from './FormattedDate'; -import GradeStatusChip from './GradeStatusChip'; +import GradeIndicator from './GradeIndicator'; import type { OrderableActivityTableColumn } from './OrderableActivityTable'; import OrderableActivityTable from './OrderableActivityTable'; @@ -208,7 +208,12 @@ export default function AssignmentActivity() { '-my-0.5', )} > - + ); default: diff --git a/lms/static/scripts/frontend_apps/components/dashboard/GradeIndicator.tsx b/lms/static/scripts/frontend_apps/components/dashboard/GradeIndicator.tsx new file mode 100644 index 0000000000..b8c66ce2e0 --- /dev/null +++ b/lms/static/scripts/frontend_apps/components/dashboard/GradeIndicator.tsx @@ -0,0 +1,129 @@ +import { CheckIcon, CancelIcon } from '@hypothesis/frontend-shared'; +import classnames from 'classnames'; +import type { ComponentChildren } from 'preact'; +import { useCallback, useState } from 'preact/hooks'; + +import type { AutoGradingConfig } from '../../api-types'; +import GradeStatusChip from './GradeStatusChip'; + +type AnnotationCountProps = { + children: ComponentChildren; + actualAmount: number; + requiredAmount: number; +}; + +function AnnotationCount({ + children, + actualAmount, + requiredAmount, +}: AnnotationCountProps) { + const requirementWasMet = actualAmount >= requiredAmount; + + return ( +
+
+ {children} +
+ {actualAmount}/{requiredAmount} +
+
+
+ {requirementWasMet ? : } +
+
+ ); +} + +export type GradeIndicatorProps = { + grade: number; + annotations: number; + replies: number; + config?: AutoGradingConfig; +}; + +/** + * Includes a GradeStatusChip, together with a popover indicating why that is + * the grade + */ +export default function GradeIndicator({ + grade, + annotations, + replies, + config, +}: GradeIndicatorProps) { + const [popoverVisible, setPopoverVisible] = useState(false); + const showPopover = useCallback(() => setPopoverVisible(true), []); + const hidePopover = useCallback(() => setPopoverVisible(false), []); + + const isCalculationSeparate = config?.activity_calculation === 'separate'; + const combined = annotations + replies; + const requiredCombined = config + ? config.required_annotations + (config.required_replies ?? 0) + : 0; + + return ( +
+ +
+ {popoverVisible && ( +
+
+ Grade calculation +
+ {isCalculationSeparate && ( + + Annotations + + )} + {isCalculationSeparate ? ( + + Replies + + ) : ( + + Annotations and replies + + )} +
+ )} +
+
+ ); +} diff --git a/lms/static/scripts/frontend_apps/components/dashboard/GradeStatusChip.tsx b/lms/static/scripts/frontend_apps/components/dashboard/GradeStatusChip.tsx index c6c0d1e77c..d36efcf2d9 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/GradeStatusChip.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/GradeStatusChip.tsx @@ -24,14 +24,19 @@ export default function GradeStatusChip({ grade }: GradeStatusChipProps) { return (
= 80 && grade < 100, - 'bg-grade-warning-light text-grade-warning': grade >= 50 && grade < 80, - 'bg-grade-error-light text-grade-error': grade >= 1 && grade < 50, - 'bg-grade-error text-white': grade === 0, - 'bg-grey-3 text-grey-7': gradeIsInvalid, - })} + className={classnames( + 'rounded inline-block font-bold px-2 py-0.5 cursor-default', + { + 'bg-grade-success text-white': grade === 100, + 'bg-grade-success-light text-grade-success': + grade >= 80 && grade < 100, + 'bg-grade-warning-light text-grade-warning': + grade >= 50 && grade < 80, + 'bg-grade-error-light text-grade-error': grade >= 1 && grade < 50, + 'bg-grade-error text-white': grade === 0, + 'bg-grey-3 text-grey-7': gradeIsInvalid, + }, + )} > {grade} {!gradeIsInvalid && '%'} diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js index 35ddf281bd..9685f853b0 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/AssignmentActivity-test.js @@ -384,9 +384,9 @@ describe('AssignmentActivity', () => { .find('OrderableActivityTable') .props() .renderItem({ auto_grading_grade }, 'auto_grading_grade'); - const gradeChip = mount(item).find('GradeStatusChip'); + const gradeIndicator = mount(item).find('GradeIndicator'); - assert.equal(gradeChip.prop('grade'), auto_grading_grade ?? 0); + assert.equal(gradeIndicator.prop('grade'), auto_grading_grade ?? 0); }); }, ); diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/GradeIndicator-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/GradeIndicator-test.js new file mode 100644 index 0000000000..44ced29c8b --- /dev/null +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/GradeIndicator-test.js @@ -0,0 +1,167 @@ +import { checkAccessibility } from '@hypothesis/frontend-testing'; +import { mount } from 'enzyme'; +import { act } from 'preact/test-utils'; + +import GradeIndicator from '../GradeIndicator'; + +describe('GradeIndicator', () => { + const defaultConfig = { + grading_type: 'all_or_nothing', + activity_calculation: 'separate', + required_annotations: 1, + required_replies: 1, + }; + + function createComponent(config = defaultConfig) { + return mount( + , + ); + } + + /** + * @param {'onMouseOver' | 'onMouseOut' | 'onFocus' | 'onBlur'} callback + */ + function invokeCallback(wrapper, callback) { + act(() => wrapper.find('[data-testid="container"]').prop(callback)()); + wrapper.update(); + } + + function openPopover(wrapper) { + invokeCallback(wrapper, 'onMouseOver'); + } + + function isPopoverVisible(wrapper) { + return wrapper.exists('[data-testid="popover"]'); + } + + ['onMouseOver', 'onFocus'].forEach(callback => { + it(`shows popover ${callback}`, () => { + const wrapper = createComponent(); + + assert.isFalse(isPopoverVisible(wrapper)); + invokeCallback(wrapper, callback); + assert.isTrue(isPopoverVisible(wrapper)); + }); + }); + + ['onMouseOut', 'onBlur'].forEach(callback => { + it(`hides popover ${callback}`, () => { + const wrapper = createComponent(); + + // Start with the popover open + openPopover(wrapper); + assert.isTrue(isPopoverVisible(wrapper)); + + invokeCallback(wrapper, callback); + assert.isFalse(isPopoverVisible(wrapper)); + }); + }); + + [ + { + config: { + activity_calculation: 'cumulative', + }, + expectedAnnotationCounts: [ + { + text: 'Annotations and replies7/2', + icon: 'CheckIcon', + }, + ], + }, + { + config: { + activity_calculation: 'cumulative', + required_annotations: 30, + required_replies: 5, + }, + expectedAnnotationCounts: [ + { + text: 'Annotations and replies7/35', + icon: 'CancelIcon', + }, + ], + }, + { + config: { + activity_calculation: 'separate', + required_annotations: 8, + required_replies: 15, + }, + expectedAnnotationCounts: [ + { + text: 'Annotations5/8', + icon: 'CancelIcon', + }, + { + text: 'Replies2/15', + icon: 'CancelIcon', + }, + ], + }, + { + config: { + activity_calculation: 'separate', + }, + expectedAnnotationCounts: [ + { + text: 'Annotations5/1', + icon: 'CheckIcon', + }, + { + text: 'Replies2/1', + icon: 'CheckIcon', + }, + ], + }, + ].forEach(({ config, expectedAnnotationCounts }) => { + it('shows expected annotation counts for config', () => { + const wrapper = createComponent({ + ...defaultConfig, + ...config, + }); + openPopover(wrapper); + + const annotationCountElements = wrapper.find('AnnotationCount'); + assert.equal( + annotationCountElements.length, + expectedAnnotationCounts.length, + ); + + expectedAnnotationCounts.forEach((expectedAnnotationCount, index) => { + const annotationCountElement = annotationCountElements.at(index); + + assert.equal( + annotationCountElement.text(), + expectedAnnotationCount.text, + ); + assert.isTrue( + annotationCountElement.exists(expectedAnnotationCount.icon), + ); + }); + }); + }); + + it( + 'should pass a11y checks', + checkAccessibility([ + { + name: 'popover closed', + content: () => createComponent(), + }, + { + name: 'popover open', + content: () => { + const wrapper = createComponent(); + openPopover(wrapper); + return wrapper; + }, + }, + ]), + ); +});