-
Notifications
You must be signed in to change notification settings - Fork 174
Improve UI/Functionality of Grading Table #332
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.
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
Changes from all commits
3786774
b00625c
ea14e1b
e8dbe3b
d045ce5
8e592b7
de00ff0
e00cd25
135dd83
02ba0ad
19256a0
b75c3bd
808043c
fe8a40b
d7aa74f
2a29283
f6819dc
861af5d
239a81b
7d28030
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import { Popover, PopoverInteractionKind, Position } from '@blueprintjs/core' | ||
|
|
||
| import * as React from 'react' | ||
|
|
||
| import { GradingOverview } from './gradingShape' | ||
|
|
||
| type GradingHistoryProps = { | ||
| data: GradingOverview | ||
| exp: boolean | ||
| grade: boolean | ||
| } | ||
|
|
||
| /** | ||
| * Used to render the marking history in the table that displays GradingOverviews. | ||
| * This is a fully fledged component (not SFC) by specification in | ||
| * ag-grid. | ||
| * | ||
| * See {@link https://www.ag-grid.com/example-react-dynamic} | ||
| */ | ||
| class GradingHistory extends React.Component<GradingHistoryProps, {}> { | ||
| constructor(props: GradingHistoryProps) { | ||
| super(props) | ||
| } | ||
|
|
||
| public render() { | ||
| const popoverInfo = () => ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't need to be a function, you can store the element as a variable
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have refactored this element based off the other suggestions below! |
||
| <div className="col-xs-12" style={{ padding: 10 }}> | ||
| {(this.props.grade && ( | ||
| <div> | ||
| <p>Initial Grade: {this.props.data.initialGrade}</p> | ||
| <p>Grade Adjustments: {this.props.data.gradeAdjustment}</p> | ||
| </div> | ||
| )) || | ||
| (this.props.exp && ( | ||
| <div> | ||
| <p>Initial XP: {this.props.data.initialXp}</p> | ||
| <p>XP Adjustments: {this.props.data.xpAdjustment}</p> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ) | ||
|
|
||
| /** Component to render in table - marks */ | ||
| const GradingMarks = () => { | ||
| if (this.props.data.maxGrade !== 0) { | ||
| return ( | ||
| <div> | ||
| {`${this.props.data.currentGrade}`} / {`${this.props.data.maxGrade}`} | ||
| </div> | ||
| ) | ||
| } else { | ||
| return <div>N/A</div> | ||
| } | ||
| } | ||
|
|
||
| /** Component to render in table - XP */ | ||
| const GradingExp = () => { | ||
| if (this.props.data.currentXp && this.props.data.maxXp) { | ||
| return ( | ||
| <div> | ||
| {`${this.props.data.currentXp}`} / {`${this.props.data.maxXp}`} | ||
| </div> | ||
| ) | ||
| } else { | ||
| return <div>No Exp</div> | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <Popover | ||
| content={popoverInfo()} | ||
| position={Position.LEFT} | ||
| interactionKind={PopoverInteractionKind.HOVER} | ||
| > | ||
| {(this.props.exp && <GradingExp />) || (this.props.grade && <GradingMarks />)} | ||
| </Popover> | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| export default GradingHistory | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { NonIdealState, Spinner } from '@blueprintjs/core' | ||
| import { Colors, InputGroup, NonIdealState, Spinner } from '@blueprintjs/core' | ||
| import { IconNames } from '@blueprintjs/icons' | ||
| import { ColDef, ColumnApi, GridApi, GridReadyEvent } from 'ag-grid' | ||
| import { ColDef, GridApi, GridReadyEvent } from 'ag-grid' | ||
| import { AgGridReact } from 'ag-grid-react' | ||
| import 'ag-grid/dist/styles/ag-grid.css' | ||
| import 'ag-grid/dist/styles/ag-theme-balham.css' | ||
|
|
@@ -12,6 +12,7 @@ import GradingWorkspaceContainer from '../../../containers/academy/grading/Gradi | |
| import { stringParamToInt } from '../../../utils/paramParseHelpers' | ||
| import { controlButton } from '../../commons' | ||
| import ContentDisplay from '../../commons/ContentDisplay' | ||
| import GradingHistory from './GradingHistory' | ||
| import GradingNavLink from './GradingNavLink' | ||
| import { GradingOverview } from './gradingShape' | ||
| import { OwnProps as GradingWorkspaceProps } from './GradingWorkspace' | ||
|
|
@@ -22,6 +23,11 @@ import { OwnProps as GradingWorkspaceProps } from './GradingWorkspace' | |
| */ | ||
| type State = { | ||
| columnDefs: ColDef[] | ||
| filterValue: string | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a few reasons I went for this change:
Could I propose having both methods of filtering available? I can re-enable this filter again. So the graders can go with whatever method is convenient for them!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update: re-enabled the filtering as well!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that sounds good |
||
| } | ||
|
|
||
| type GradingNavLinkProps = { | ||
| data: GradingOverview | ||
| } | ||
|
|
||
| interface IGradingProps | ||
|
|
@@ -42,33 +48,63 @@ export interface IStateProps { | |
| gradingOverviews?: GradingOverview[] | ||
| } | ||
|
|
||
| /** Component to render in table - marks */ | ||
| const GradingMarks = (props: GradingNavLinkProps) => { | ||
| return <GradingHistory data={props.data} exp={false} grade={true} /> | ||
| } | ||
|
|
||
| /** Component to render in table - XP */ | ||
| const GradingExp = (props: GradingNavLinkProps) => { | ||
| return <GradingHistory data={props.data} exp={true} grade={false} /> | ||
| } | ||
|
|
||
| class Grading extends React.Component<IGradingProps, State> { | ||
| private gridApi?: GridApi | ||
| private columnApi?: ColumnApi | ||
|
|
||
| public constructor(props: IGradingProps) { | ||
| super(props) | ||
|
|
||
| this.state = { | ||
| columnDefs: [ | ||
| { headerName: 'Assessment ID', field: 'assessmentId' }, | ||
| { headerName: 'Assessment Name', field: 'assessmentName' }, | ||
| { headerName: 'Assessment Category', field: 'assessmentCategory' }, | ||
| { headerName: 'Category', field: 'assessmentCategory', maxWidth: 150 }, | ||
| { headerName: 'Student Name', field: 'studentName' }, | ||
| { headerName: 'Auograder grade', field: 'initialGrade' }, | ||
| { headerName: 'Grade adjustment', field: 'gradeAdjustment' }, | ||
| { headerName: 'Current Grade', field: 'currentGrade' }, | ||
| { headerName: 'Maximum Grade', field: 'maxGrade' }, | ||
| { headerName: 'XP', field: 'initialXp' }, | ||
| { headerName: 'XP adjustment', field: 'xpAdjustment' }, | ||
| { headerName: 'Current XP', field: 'currentXp' }, | ||
| { headerName: 'Maximum XP', field: 'maxXp' }, | ||
| { | ||
| headerName: 'Grade', | ||
| field: '', | ||
| cellRendererFramework: GradingMarks, | ||
| maxWidth: 100, | ||
| cellStyle: params => { | ||
| if (params.data.currentGrade < params.data.maxGrade) { | ||
| return { backgroundColor: Colors.RED5 } | ||
| } else { | ||
| return {} | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| headerName: 'XP', | ||
| field: '', | ||
| cellRendererFramework: GradingExp, | ||
| maxWidth: 100 | ||
| }, | ||
| { | ||
| headerName: 'Edit', | ||
| field: '', | ||
| cellRendererFramework: GradingNavLink | ||
| } | ||
| ] | ||
| cellRendererFramework: GradingNavLink, | ||
| maxWidth: 70 | ||
| }, | ||
| { headerName: 'Initial Grade', field: 'initialGrade', hide: true }, | ||
| { headerName: 'Grade Adjustment', field: 'gradeAdjustment', hide: true }, | ||
| { headerName: 'Initial XP', field: 'initialXp', hide: true }, | ||
| { headerName: 'XP Adjustment', field: 'xpAdjustment', hide: true }, | ||
| { headerName: 'Current Grade', field: 'currentGrade', hide: true }, | ||
| { headerName: 'Max Grade', field: 'maxGrade', hide: true }, | ||
| { headerName: 'Current XP', field: 'currentXp', hide: true }, | ||
| { headerName: 'Max XP', field: 'maxXp', hide: true } | ||
| ], | ||
|
|
||
| filterValue: '' | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -98,44 +134,69 @@ class Grading extends React.Component<IGradingProps, State> { | |
| (a: GradingOverview) => -a.assessmentId, | ||
| (a: GradingOverview) => -a.submissionId | ||
| ]) | ||
|
|
||
| const grid = ( | ||
| <div className="Grading"> | ||
| <div className="ag-grid-parent ag-theme-balham"> | ||
| <AgGridReact | ||
| gridAutoHeight={true} | ||
| enableColResize={true} | ||
| enableSorting={true} | ||
| enableFilter={true} | ||
| columnDefs={this.state.columnDefs} | ||
| onGridReady={this.onGridReady} | ||
| rowData={data} | ||
| /> | ||
| <div className="GradingContainer"> | ||
| <div> | ||
| <div className="col-md-6 col-md-offset-3"> | ||
| <InputGroup | ||
| large={false} | ||
| leftIcon="filter" | ||
| placeholder="Filter..." | ||
| value={this.state.filterValue} | ||
| onChange={this.handleFilterChange} | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="ag-grid-export-button"> | ||
| {controlButton('Export to CSV', IconNames.EXPORT, this.exportCSV)} | ||
|
|
||
| <br /> | ||
|
|
||
| <div className="Grading"> | ||
| <div className="ag-grid-parent ag-theme-fresh"> | ||
| <AgGridReact | ||
| gridAutoHeight={true} | ||
| enableColResize={true} | ||
| enableSorting={true} | ||
| enableFilter={true} | ||
| columnDefs={this.state.columnDefs} | ||
| onGridReady={this.onGridReady} | ||
| rowData={data} | ||
| /> | ||
| </div> | ||
| <div className="ag-grid-export-button"> | ||
| {controlButton('Export to CSV', IconNames.EXPORT, this.exportCSV)} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| return ( | ||
| <ContentDisplay | ||
| loadContentDispatch={this.props.handleFetchGradingOverviews} | ||
| display={this.props.gradingOverviews === undefined ? loadingDisplay : grid} | ||
| fullWidth={true} | ||
| fullWidth={false} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| private handleFilterChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const changeVal = event.target.value | ||
| this.setState({ filterValue: changeVal }) | ||
|
|
||
| if (this.gridApi) { | ||
| this.gridApi.setQuickFilter(changeVal) | ||
| } | ||
| } | ||
|
|
||
| private onGridReady = (params: GridReadyEvent) => { | ||
| this.gridApi = params.api | ||
| this.columnApi = params.columnApi | ||
| this.columnApi.autoSizeAllColumns() | ||
| this.gridApi.sizeColumnsToFit() | ||
| } | ||
|
|
||
| private exportCSV = () => { | ||
| if (this.gridApi === undefined) { | ||
| return | ||
| } | ||
| this.gridApi.exportDataAsCsv() | ||
| this.gridApi.exportDataAsCsv({ allColumns: true }) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,7 +176,12 @@ class Assessment extends React.Component<IAssessmentProps, State> { | |
| <p> | ||
| You are about to finalise your submission for the{' '} | ||
| {this.state.betchaAssessment.category.toLowerCase()}{' '} | ||
| <i>"{this.state.betchaAssessment.title}"</i>. | ||
| <i> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a formatter issue?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. I believe |
||
| " | ||
| {this.state.betchaAssessment.title} | ||
| " | ||
| </i> | ||
| . | ||
| </p> | ||
| <p> | ||
| Early submissions grant you additional XP, but{' '} | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you describe the issue you had with travis? It's not that obvious to me from seeing this. (I don't mean to document it just yet, you can reply here 😃 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 test suite -
Assessmentsactually fails on Travis (some issue with the snapshot I believe)On my local system, I managed to resolve this by upgrading
Yarnto version 1.9. I tried the same with Travis but it didnt seem to work either.Should we open an issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test probably failed because there was a changed made to the component. I'll update the tests