Improve UI/Functionality of Grading Table #332

rrtheonlyone · 2018-08-25T03:29:56Z

As it stands right now, it not very convenient for the graders to mark/comment on their student's assignments. It is important that the grading process is more seamless so that the graders will be more proactive in grading their students. This PR addresses this issue.

The fixes in the PR:

Everything is on one screen (no horizontal scrolling)
Unnecessary Information is removed and UI is cleaned up
Popover to show history (grade adjustments)
Editing a student's assignment opens in a new tab - no need to go back and forth
Quick filter input to filter anything in the table (by name/'mission' etc.)
Coloured Cell to show that a student has not achieved full marks
'N/A' or 'No exp' shown when mission doesn't have grades

How it looks:

coveralls · 2018-08-25T09:46:21Z

Pull Request Test Coverage Report for Build 757

9 of 34 (26.47%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 26.095%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/academy/grading/GradingHistory.tsx	3	15	20.0%
src/components/academy/grading/index.tsx	4	17	23.53%

Totals
Change from base Build 724:	-0.1%
Covered Lines:	894
Relevant Lines:	2921

💛 - Coveralls

remo5000

Nice work! I've only looked at the code so far though, will let you know if there's anything to change regarding visuals

remo5000 · 2018-08-25T09:45:43Z

src/components/academy/grading/GradingHistory.tsx

+  }
+
+  public render() {
+    const popoverInfo = () => (


This doesn't need to be a function, you can store the element as a variable

I have refactored this element based off the other suggestions below!

remo5000 · 2018-08-25T09:46:50Z

.travis.yml

 node_js:
  - 9
-cache: yarn
+before_install:


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 😃 )

1 test suite - Assessments actually fails on Travis (some issue with the snapshot I believe)

On my local system, I managed to resolve this by upgrading Yarn to version 1.9. I tried the same with Travis but it didnt seem to work either.

Should we open an issue for this?

The test probably failed because there was a changed made to the component. I'll update the tests

remo5000 · 2018-08-25T09:50:55Z

src/components/academy/grading/index.tsx

+          maxWidth: 100,
+          cellStyle: params => {
+            if (params.data.currentGrade < params.data.maxGrade) {
+              return { backgroundColor: '#ffe6e6' }


Great idea for highlighting the colour -- could you define the background colour as a constant at the top of the file? Better yet, you could use Blueprint's Intent colour, that would be preferred

Done! I changed it to use Blueprint's Intent Colour

remo5000 · 2018-08-25T10:08:50Z

src/components/assessment/index.tsx

          You are about to finalise your submission for the{' '}
          {this.state.betchaAssessment.category.toLowerCase()}{' '}
-          <i>&quot;{this.state.betchaAssessment.title}&quot;</i>.
+          <i>


Is this a formatter issue?

Yup. I believe yarn format changed it as such

remo5000 · 2018-08-25T10:09:53Z

src/utils/constants.ts

 export const IVLE_KEY = process.env.REACT_APP_IVLE_KEY
 export const VERSION = process.env.REACT_APP_VERSION
 export const BACKEND_URL = process.env.REACT_APP_BACKEND_URL
-export const USE_BACKEND = process.env.REACT_APP_USE_BACKEND


Please change this back to the original value, you can set this variable using the .env file. If you don't have one, you can make a copy of it from .env.example

Oh man. Yup am aware of .env.

I changed it to try something and accidentally pushed it in (sry)

I changed it back

remo5000

I've outlined 2 design changes -- the column one in particular is of most importance 👍

remo5000 · 2018-08-25T10:17:14Z

src/components/academy/grading/index.tsx

 */
 type State = {
  columnDefs: ColDef[]
+  filterValue: string


Can we stick to the original method of filtering? IMO it was much more versatile, as we could filter out individual columns:

There are a few reasons I went for this change:

Even if it was more versatile, from a UI point of view it was not user-friendly e.g. need to choose from dropdown + input text and it wasn't obvious either (I wasnt even aware of this feature till I saw the source code)

It actually doesn't look good either especially with the new theme I have used.

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!

Update: re-enabled the filtering as well!

Sure, that sounds good

remo5000 · 2018-08-25T10:18:36Z

src/components/academy/grading/index.tsx

-import { NonIdealState, Spinner } from '@blueprintjs/core'
+import { InputGroup, NonIdealState, Spinner } from '@blueprintjs/core'
 import { IconNames } from '@blueprintjs/icons'
-import { ColDef, ColumnApi, GridApi, GridReadyEvent } from 'ag-grid'


if we are getting rid of resizable columns, could the column sizes be specified (in a relative format). We can space out Grade, XP, edit and history a bit more. This isn't that critical though -- so if it's nontrivial, you can leave it out.

Yup in the code I have specified maxWidth property for certain columns (so it stretches accordingly)

remo5000 · 2018-08-25T10:24:19Z

src/components/academy/grading/GradingHistory.tsx

+ *
+ * See {@link https://www.ag-grid.com/example-react-dynamic}
+ */
+class GradingHistory extends React.Component<GradingNavLinkProps, {}> {


I like the idea of this column, but I think there's an even more elegant way. Could you refactor this component to be used for the Grade and XP columns? What I mean is instead of having a separate "history" section, one can see the initial grade/xp and adjustment when they hover over the grade. If my explanation is unclear do let me know 😅

Yup I know what you mean 👍

I have refactored the component and made it as you requested! It is much cleaner now 😄

remo5000

I like the new grading columns! Just encountered some weird behaviour:

Filter doesn't reset when the filter text is erased
Export to CSV doesn't show grade and XP
Highlighting of the grade makes the text look off-center. Could the text be highlighted instead? (Sorry for the lack of oversight on this)

…r the grade/exp itself

rrtheonlyone · 2018-08-26T11:02:25Z

Do check it out now:

Fixed CSV Import bug
Filter should now change table instantly
Everything is more centralised (it wasnt centralised before this - the colouring had nothing to do with it)

remo5000

Thanks for the quick response!

Filter doesn't seem to filter out mission names anymore, is this because of the latest fix? If so, can it be enabled?
Could the filter default text say what column it will filter? It is not obvious at first glance
Is it possible to remove the Grade, XP and Edit columns from the excel? Being a bit knitpicky, but if this is a nontrivial fix then you can exclude it for now

rrtheonlyone · 2018-08-27T12:02:20Z

The filter is a quick filter that is supposed to filter anything in the table.

It works for me though - can you give me an example of which case didnt work? @remo5000

The CSV Import may require additional work - will have to log that as a separate issue. I cannot seem to find anything on the API to exclude certain columns.

* Update backend parsing Fix lint error * Modify grading shape * Use new properties for grading overview table * Update mock grading api * Update table field * Update text for editing grading * Sort data by assessmentId, then submissionId * Format file * Improve UI/Functionality of Grading Table (#332) * Improved Table UI and made functionality more intuitive * Improved UI for grading table and made functionality more intutive * Added comments and ran yarn format * Ran Yarn format to fix all linting issues * Formating Change and Fixing Conflicts * Upgraded yarn version on TRAVIS * Attempt to remove cache yarn * Clean up formatting (again) * Fix rebase error * Used blueprintJS for Color instead of hex code * Modified constant to original value * Reenabled sorting for columns && refactored history to show popup over the grade/exp itself * Update formatting - prettify * Fix bug in filter * Fix CSV export - show all required details * update formatting * Centralise and vertically align content in grading table * Fixed formatting for scss file * Fix bug in grade display * Centralised filter input

* Add XP to profile (duplicate) (#323) * Add maxGrade and xp to state * Pass props to Profile from State * Remove IS_XP_IMPLEMENTED * Unhide betcha * Use xp and maxgrade for Profile * Update tests and format * Xp assessment (#320) * Update IAssessmentOverview Sticking to how the backend specifies the props, since it's already camel case * Remove maxGrade transform (IAssessmentOverview) * Use maxGrade in assesment cards * Update mock assessment data * Add grade and xp into assessment overview * Hide grade and xp for unopened assessments * Show open date for unopened assessments * Format and update tests * Add XP for grading overview (#321) * Update backend parsing Fix lint error * Modify grading shape * Use new properties for grading overview table * Update mock grading api * Update table field * Update text for editing grading * Sort data by assessmentId, then submissionId * Format file * Improve UI/Functionality of Grading Table (#332) * Improved Table UI and made functionality more intuitive * Improved UI for grading table and made functionality more intutive * Added comments and ran yarn format * Ran Yarn format to fix all linting issues * Formating Change and Fixing Conflicts * Upgraded yarn version on TRAVIS * Attempt to remove cache yarn * Clean up formatting (again) * Fix rebase error * Used blueprintJS for Color instead of hex code * Modified constant to original value * Reenabled sorting for columns && refactored history to show popup over the grade/exp itself * Update formatting - prettify * Fix bug in filter * Fix CSV export - show all required details * update formatting * Centralise and vertically align content in grading table * Fixed formatting for scss file * Fix bug in grade display * Centralised filter input * Add XP adjustment input to Grading Workspace (#335) * Add parsing for new grading schema * Add xp props to GradingQuestion * Fix props in GradingWorkspace * Update mock data * Update mock data props * Add props to saga * Fix rebase errors * Reorder props in type * Add xp adjustment to action * Rename dispatch function arguments * Modify adjustment-related props * Add xp OwnProps Renamed a few usage of the props as well * Add xp to validation and submit dispatch call * Rename more props usage * Duplicate some columns for XP Most importantly, the numeric input * Remove comment * Pass in xp props to GradingEditor * Add Final XP column * Add xp and grade adjustment to mock backend * Remove old TODO comment * Use correct arguments for backend grading submit * Use correct arguments for postGrading * Format files * Fix runtime errors - input error when min === max - error message - hasUnsavedChanges returning true because of xp checking against grade * Format file * Revert travis to master's version * Show programming solution (#336) * Add additional types for solution prop * Udpate mock data * Use markdown to show solution * Pass solution to GradingEditor * Use code tag for solution instead * Add parsing of solution in saga * Add failsafes for solution value and the maxGrade value as well. This is something that only happens in staging, but could very well happen in prod as well. * Use pre for solution And fix a bug with xpAdjustment * Add word wrapping to solution * Fix wrapping for pre tag It was not speicific to the pre tag * Format files

Bumps [excoveralls](https://github.com/parroty/excoveralls) from 0.10.4 to 0.10.5. - [Release notes](https://github.com/parroty/excoveralls/releases) - [Changelog](https://github.com/parroty/excoveralls/blob/master/CHANGELOG.md) - [Commits](parroty/excoveralls@v0.10.4...v0.10.5) Signed-off-by: dependabot[bot] <support@dependabot.com>

rrtheonlyone requested review from indocomsoft and remo5000 August 25, 2018 03:30

rrtheonlyone added 8 commits August 25, 2018 17:41

Improved Table UI and made functionality more intuitive

3786774

Improved UI for grading table and made functionality more intutive

b00625c

Added comments and ran yarn format

ea14e1b

Ran Yarn format to fix all linting issues

e8dbe3b

Formating Change and Fixing Conflicts

d045ce5

Upgraded yarn version on TRAVIS

8e592b7

Attempt to remove cache yarn

de00ff0

Clean up formatting (again)

e00cd25

remo5000 force-pushed the grading-table-upgrade branch from 90ab593 to e00cd25 Compare August 25, 2018 09:42

remo5000 changed the base branch from master to xp August 25, 2018 09:43

remo5000 changed the base branch from xp to xp-grading-overview August 25, 2018 09:43

remo5000 requested changes Aug 25, 2018

View reviewed changes

remo5000 added the blocking Finishing this opens up other stuff label Aug 25, 2018

remo5000 requested changes Aug 25, 2018

View reviewed changes

Fix rebase error

135dd83

remo5000 requested changes Aug 26, 2018

View reviewed changes

rrtheonlyone added 4 commits August 26, 2018 18:17

Used blueprintJS for Color instead of hex code

02ba0ad

Modified constant to original value

19256a0

Reenabled sorting for columns && refactored history to show popup ove…

b75c3bd

…r the grade/exp itself

Update formatting - prettify

808043c

rrtheonlyone force-pushed the grading-table-upgrade branch from 1365647 to 808043c Compare August 26, 2018 10:42

rrtheonlyone added 5 commits August 26, 2018 18:46

Fix bug in filter

fe8a40b

Fix CSV export - show all required details

d7aa74f

update formatting

2a29283

Centralise and vertically align content in grading table

f6819dc

Fixed formatting for scss file

861af5d

remo5000 requested changes Aug 27, 2018

View reviewed changes

Fix bug in grade display

239a81b

remo5000 approved these changes Aug 28, 2018

View reviewed changes

Centralised filter input

7d28030

remo5000 merged commit 7c2aa5c into xp-grading-overview Aug 28, 2018

rrtheonlyone deleted the grading-table-upgrade branch September 1, 2018 13:52

Improve UI/Functionality of Grading Table #332

Improve UI/Functionality of Grading Table #332

Uh oh!

Conversation

rrtheonlyone commented Aug 25, 2018

Uh oh!

coveralls commented Aug 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 757

💛 - Coveralls

Uh oh!

remo5000 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remo5000 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rrtheonlyone Aug 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remo5000 left a comment

Choose a reason for hiding this comment

Uh oh!

rrtheonlyone commented Aug 26, 2018

Uh oh!

remo5000 left a comment

Choose a reason for hiding this comment

Uh oh!

rrtheonlyone commented Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Aug 25, 2018 •

edited

Loading

remo5000 left a comment •

edited

Loading

remo5000 left a comment •

edited

Loading

rrtheonlyone Aug 26, 2018 •

edited

Loading

rrtheonlyone commented Aug 27, 2018 •

edited

Loading