Skip to content
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

Fixes crashes in development mode #2

Merged
merged 1 commit into from
Mar 6, 2019
Merged

Conversation

NikhilM98
Copy link

@NikhilM98 NikhilM98 commented Feb 28, 2019

Closes issue #6. The color value was not handled properly which was causing the react app to throw an error that cannot read property of undefined. According to the original developer, this app is not expected to function properly in development mode due to dependence on Sugarizer and Sugarizer Neighborhood data but I still think that result screen is a crucial functionality of the app and it should not break at the result screen even at the development mode.

@NikhilM98
Copy link
Author

P.S. @llaske I'm participating in GSoC'19 and I want to discuss my ideas regarding Sugarizer, Exerciser, and other activities.

@@ -10,7 +10,7 @@ import {
CHOOSE,
CLOZE_TEXT,
MCQ,
REORDER_LIST, TITLE_OF_EXERCISE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this change is not related to this issue

@@ -1,8 +1,7 @@
import React, {Component} from "react"
import {Bar, Line, Pie} from 'react-chartjs-2';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this change is not related to this issue

import {withRouter} from "react-router-dom";
import {connect} from "react-redux";
import {addScore} from "../../store/actions/exercises";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this change is not related to this issue

@llaske
Copy link
Owner

llaske commented Mar 6, 2019

Not a bad idea to solve this issue to allow work in dev mode.

Could you reopen the issue on this repo to reference it here?

Plus, your PR include code update not related to this fix (see my comments). Could you clean it?

@NikhilM98
Copy link
Author

@llaske I've made the changes you asked for.
There are some components which are imported in the code but are not being used. It would be inefficient to import such components. I've also made some other suggestions here. Can you please review them.

@llaske
Copy link
Owner

llaske commented Mar 6, 2019

Okay thanks. Will review your suggestions.

@llaske llaske merged commit d4af434 into llaske:master Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants