Grading comments #162

remo5000 · 2018-07-05T05:59:52Z

Features

Rich-text markdown editor to add comments
Slice of state within WorkspaceManager for storing gradingComments
Comments updated when component unmounts, as compared to updating on change. This is because
- The editor requires a local state, so it is simpler to set the local state in the constructor, using StateProps, and update the store when unmounting using a dispatch
- Comments do not require evaluation. The button to save the comments (and update XP) will be implemented by reading local state, which already has the updated text in it.

TODO

number input for XP
button to submit comments and XP

coveralls · 2018-07-05T06:05:12Z

Pull Request Test Coverage Report for Build 210

33 of 79 (41.77%) changed or added relevant lines in 12 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.007%) to 31.519%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/sagas/index.ts	1	2	50.0%
src/components/assessment/index.tsx	1	2	50.0%
src/components/academy/grading/GradingNavLink.tsx	3	5	60.0%
src/actions/workspaces.ts	4	6	66.67%
src/containers/academy/grading/GradingEditorContainer.ts	7	9	77.78%
src/reducers/workspaces.ts	0	3	0.0%
src/components/academy/grading/index.tsx	1	8	12.5%
src/components/academy/grading/GradingEditor.tsx	8	36	22.22%

Totals
Change from base Build 208:	-0.007%
Covered Lines:	1237
Relevant Lines:	3445

💛 - Coveralls

remo5000 · 2018-07-05T09:15:34Z

src/actions/actionTypes.ts

+export const UPDATE_CURRENT_SUBMISSION_ID = 'UPDATE_CURRENT_SUBMISSION_ID'
+export const UPDATE_GRADING_COMMENTS_VALUE = 'UPDATE_GRADING_COMMENTS_VALUE'
+export const UPDATE_GRADING_XP = 'UPDATE_GRADING_XP'
+export const SAVE_GRADING_INPUT = 'SAVE_GRADING_INPUT'


Should I change this to UPLOAD_GRADING_INPUT instead? It's only use is to call a saga.

I think save is fine

remo5000 · 2018-07-05T09:16:40Z

src/components/academy/grading/GradingEditor.tsx

+
+  constructor(props: GradingEditorProps) {
+    super(props)
+    this.state = {


Local state is used to keep track of everything here, because the editor already uses it. Upon unmounting, the values are dispatched.

remo5000 · 2018-07-05T09:26:10Z

src/components/assessment/index.tsx

  public componentWillMount() {
    const assessmentId = stringParamToInt(this.props.match.params.assessmentId)
-    const questionId = stringParamToInt(this.props.match.params.questionId)
-    if (assessmentId === null || questionId === null) {


This fixes a bug that applies for Grading but not Assessment. I changed it for consistency.

The original predicate fails when questionid is null but submissionId is non-null (because we have not assigned 0 to it yet). In the case of assessments, the 0 value is provided by the card, since it must provide a value in the link within.

remo5000 · 2018-07-05T09:27:08Z

src/styles/_workspace.scss

          /* word-wrap and word-break are added to make text wrap. */
          word-wrap: break-word;
          word-break: break-word;
+          .grading-editor-input-parent {


I can move this to another file if needed. I didn't because grading editor is present only inside a side-content

I think this is fine

ning-y · 2018-07-06T04:42:36Z

src/actions/actionTypes.ts

+export const UPDATE_CURRENT_SUBMISSION_ID = 'UPDATE_CURRENT_SUBMISSION_ID'
+export const UPDATE_GRADING_COMMENTS_VALUE = 'UPDATE_GRADING_COMMENTS_VALUE'
+export const UPDATE_GRADING_XP = 'UPDATE_GRADING_XP'
+export const SAVE_GRADING_INPUT = 'SAVE_GRADING_INPUT'


I think save is fine

ning-y · 2018-07-06T04:45:19Z

src/styles/_workspace.scss

          /* word-wrap and word-break are added to make text wrap. */
          word-wrap: break-word;
          word-break: break-word;
+          .grading-editor-input-parent {


I think this is fine

- ~400 stars - In Typescript - Vertical layout for the slightly narrow side-content section - In actual Markdown Only caveats are that it is not WYSIWYG and does not have a markdown to html converter.

The main library react-mde runs on.

To convert markdown to HTML and vice-versa

I'll remove this to use blueprint icons instead, later on.

Or else, the styles do not work.

Trying to use componentDidMount and componentWillUnmount to handle updates.

Alphabetisizing, typos.

The `markdown` value in `mdeState` being undefined causes a runtime error. So it is easier to just deal with a definite string (since undefined is not allowed anyway). The typedef for the value allows undefined though.

The problem is that the current <a href> implementation is a hard redirect, causing a change in URL (and loss of all state). I'm trying to get a NavLink

Since it will be set by the default state, it is never null.

Checks only for submissionId, if it is present then there should be a questionId as well.

Used to reset the workspace as well.

The issue has been created at the backend cadet repo -- #162. As triple equality was used to verify if the button should be pressed, this caused all the equality checks to return false (string is not === any number). I've added a workaround to parse the received string.

* Add handleOptionSelect handler for MCQChooser * Use handleOptionSelect in Button Had to use a curried function to get around the "Lambdas forbidden in rendering method" rule (and consequently save performance) * Use handleMCQSubmit name instead * Change mcq question prop -> mcqProps Thus, anything that calls workspace will have to provide a method to handle MCQs, if the question is an MCQ question * Pass correct props for GradingWorkspace * Use correct props for AssessmentWorkspace * Remove unused import from workspace * Format AssessmentWorkspace * Add localState to remove visual lag of choosing Before: Option chosen (active momentarily) -> dispatch (button inactive due to current state) -> component updated with answer (active) Now: Constructor takes in current option from state -> Option chosen (active momentarily) -> local state updated (active) and dispatch is sent -> any update from the dispatch will cause constructor to set the state (during render) to the new option * Pass in chosenOption prop from workspace parents * Format and update a test * Add workaround for mcq being passed as string The issue has been created at the backend cadet repo -- #162. As triple equality was used to verify if the button should be pressed, this caused all the equality checks to return false (string is not === any number). I've added a workaround to parse the received string. * Move parsing to the backend saga * Format files * Make resetWorkspace accept arguments to set values * Export IWorkspaceState * Change reducer for RESET_WORKSPACE * Modify workspace parent containers * Modify component handler definitions * Use handleResetWorkspace to set editorValue * Dont show save button for mcq question * Format files * Remove parsing * Simplify chosenMCQAnswer expression * Remove chosenMCQAnswer I did not realise that this can just be used from the IMCQQuestion passed in. * Add mock call for SUBMIT_ANSWER * Format files

remo5000 commented Jul 5, 2018

View reviewed changes

ning-y approved these changes Jul 6, 2018

View reviewed changes

remo5000 added 27 commits July 6, 2018 12:55

Added react-md -- a Markdown editor

b024ae3

- ~400 stars - In Typescript - Vertical layout for the slightly narrow side-content section - In actual Markdown Only caveats are that it is not WYSIWYG and does not have a markdown to html converter.

Add draft-js dependency

5275bdb

The main library react-mde runs on.

Add types for draft-js

7fa8ef0

Add showdown dependency

7cd2f97

To convert markdown to HTML and vice-versa

Add awesomefonts dependency

a69951e

I'll remove this to use blueprint icons instead, later on.

Add typedefs for showdown

b45a7f9

Add GradingEditor component

ffb6eb3

Add react-mde styles into index.scss

6eb14fb

Or else, the styles do not work.

Fix overflow for react-mde in side-content card

da96de0

Fix scrolling problems using css

8724bf1

Add coloring styles

de7fc50

Add dispatch and state props

0744368

Trying to use componentDidMount and componentWillUnmount to handle updates.

Add container

5cc14c2

Add action and reducer

c4df92d

Fix compiler errors

9663b27

Alphabetisizing, typos.

Remove undefined union for gradingCommentsValue

8d49ea2

The `markdown` value in `mdeState` being undefined causes a runtime error. So it is easier to just deal with a definite string (since undefined is not allowed anyway). The typedef for the value allows undefined though.

Scaffold link component to render for grid

86bfa7f

The problem is that the current <a href> implementation is a hard redirect, causing a change in URL (and loss of all state). I'm trying to get a NavLink

Format and de-style NavLink

759793d

Use undefined check

e734b8c

Force non-null for markdown value

a65ec55

Since it will be set by the default state, it is never null.

Add componentWillMount for grading/index

ef89a07

Add props for updating ids

33064f5

Add action

cdf5b63

Fix buggy componentWillMount functions

b6fa3e1

Checks only for submissionId, if it is present then there should be a questionId as well.

Add defaultComments value

78c76c0

Used to reset the workspace as well.

Format files

e59a4e2

Add NumericInput for GradingEditor

9106590

remo5000 added 12 commits July 6, 2018 12:57

Add NumericInput and Button

91720dd

Add local state property to track XP given

43efb67

Add update for grading XP

7407d1d

Add action for saving grading input

e457bc1

Add saga for saving grading

f11e3de

Format files

861ff5a

Use blueprintjs icons instead of fontawesome

409e0cd

Fix save button wrap issue

14196ff

Format files

705a2e3

Remove blank line

b401b49

Add some comments

2e52bdc

Format comments

4d563dd

remo5000 force-pushed the grading-comments branch from 714f3a2 to 4d563dd Compare July 6, 2018 04:58

remo5000 mentioned this pull request Jul 6, 2018

Tabs do not work #174

Closed

remo5000 merged commit 80482b4 into master Jul 6, 2018

remo5000 mentioned this pull request Jul 6, 2018

Fix tabs not working #175

Merged

ning-y deleted the grading-comments branch July 17, 2018 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Grading comments #162

Grading comments #162

Uh oh!

remo5000 commented Jul 5, 2018

Uh oh!

coveralls commented Jul 5, 2018 •

edited

Loading

Uh oh!

remo5000 Jul 5, 2018

Uh oh!

ning-y Jul 6, 2018

Uh oh!

remo5000 Jul 5, 2018

Uh oh!

remo5000 Jul 5, 2018

Uh oh!

remo5000 Jul 5, 2018

Uh oh!

ning-y Jul 6, 2018

Uh oh!

ning-y Jul 6, 2018

Uh oh!

ning-y Jul 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Grading comments #162

Grading comments #162

Uh oh!

Conversation

remo5000 commented Jul 5, 2018

Features

TODO

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 210

💛 - Coveralls

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jul 5, 2018 •

edited

Loading