Allow votes from the user frontend #283
Conversation
…for fields" This reverts commit 6eef631.
|
api and serializer lgtm |
The weight ist updated from the questions relative position in the question list.
There was a problem hiding this comment.
It's a lot of stuff, but most is just small things. I wanted to be thorough since you were eager to learn React. I think there still are a few anti patterns (I spent a few hours figuring out how to reduce the 11 props to QuestionForm) but was only partially convinced of any approach to that.
We can go through the points together too, or through some of them, if you'd like.
Still, nice work for a first shot!
| MISSING FORM | ||
| {% react_poll_form object %} | ||
|
|
||
| </form> |
| @@ -0,0 +1,18 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
I personally prefer to name the migrations with -n [name], but I've seen other automagically named migrations, so that's fine too, I guess.
There was a problem hiding this comment.
i'll leave the generated title this time but will name it manually next time.
maybe we could also squash them when the feature is ready
apps/polls/assets/PollManagement.jsx
Outdated
|
|
||
| getNewQuestion: function (label) { | ||
| var newQuestion = {} | ||
| newQuestion['label'] = label |
There was a problem hiding this comment.
make this
return {
label: label,
key: this.getNextQuestionKey(),
choices: []
}
apps/polls/assets/PollManagement.jsx
Outdated
| */ | ||
| var questionKey = 'local_' + (this.state.maxQuestionKey + 1) | ||
| this.setState({maxQuestionKey: this.state.maxQuestionKey + 1}) | ||
| return questionKey |
There was a problem hiding this comment.
Does this need to be in state? Should changes to the maxQuestionKey cause a render?
There was a problem hiding this comment.
good point, i changed it to be a class variable. the changes should be reflected in documents, too
apps/polls/assets/PollManagement.jsx
Outdated
| var newChoice = {} | ||
| newChoice['label'] = label | ||
| newChoice['key'] = this.getNextChoiceKey() | ||
| return newChoice |
There was a problem hiding this comment.
return {
label: label,
key: this.getNextChoiceKey()
}
apps/polls/assets/QuestionForm.jsx
Outdated
| {django.gettext('Question:')} | ||
| </label> | ||
| <input | ||
| className="form-control" |
There was a problem hiding this comment.
.form-control is of no importance anymore, we removed it. I know, theoretically the same goes for documents.
apps/polls/assets/PollManagement.jsx
Outdated
| promise | ||
| .done(function (data) { | ||
| this.setState({ | ||
| successMessage: django.gettext('The poll has been updated.') |
There was a problem hiding this comment.
You'll want to reset previous errors here.
apps/polls/assets/PollManagement.jsx
Outdated
| }.bind(this)) | ||
| .fail(function (xhr, status, err) { | ||
| this.setState({ | ||
| errors: xhr.responseJSON.questions || [] |
There was a problem hiding this comment.
Why a fallback? If there are no errors to set (which errors: [] would mean) you don't want to call setState.
apps/polls/assets/PollManagement.jsx
Outdated
| successMessage: '' | ||
| }) | ||
| }.bind(this), 1500) | ||
| }.bind(this)) |
There was a problem hiding this comment.
Consider using fat arrow functions to get rid of all the .binds
apps/polls/assets/Question.jsx
Outdated
| choice: this.props.question.choices[newChoice].id | ||
| } | ||
|
|
||
| return api.poll.vote(submitData, this.props.question.id) |
There was a problem hiding this comment.
No need to return the promise.
This PR includes the changes from #273
API changes depend on liqd/adhocracy4#89
It is missing