Fixed editing recurrences #174
Conversation
…ckend issue #210, but it's definitely better than it was.
| @@ -28,7 +28,7 @@ rules: | |||
| no-alert: warn | |||
| no-mixed-operators: warn | |||
| no-param-reassign: warn | |||
| no-plusplus: warn | |||
| no-plusplus: off | |||
There was a problem hiding this comment.
Why would we want this? It ends up highlighting for loops.
There was a problem hiding this comment.
It's from the Airbnb style guide, included by the airbnb preset.
My approach to the eslint file was to use the Airbnb preset, and then override its errors with warnings to buy us time to fix them.
I don't have a strong opinion on disabling particular warnings. ESlint's rationale for this one seems weak, given that the code is being babelized anyway.
That said, a weaker alternative to off is [warn, allowForLoopAfterthoughts: true] “allows unary operators ++ and -- in the afterthought (final expression) of a for loop."
src/pages/add-edit/add-edit-page.jsx
Outdated
| // If the labels are null/undefined, create an empty list in our state | ||
| if (!eventData.labels) { eventData.labels = []; } | ||
| const { eventData } = this.state; | ||
| if (this.state.eventData) { |
There was a problem hiding this comment.
this.state.eventData -> eventData ?
src/pages/add-edit/add-edit-page.jsx
Outdated
|
|
||
| descriptionChanged = description => this.updateEventDatum({ description }); | ||
| arraysEqual = (arr1, arr2) => { |
There was a problem hiding this comment.
The project already has a runtime dependency on lodash (because this is used by async, react-tag-input, input-moment, react-dnd, react-redux, redux, and react-tag-input), so you could also use _.isEqual.
| ); | ||
| } | ||
|
|
||
| const editingExisting = this.state.eventData.id || this.state.eventData.sid; |
There was a problem hiding this comment.
Note that editingExisting has a type string | undefined. (This isn't quite the same as Flow's ?string, because undefined !== null.) I think that's okay the way it's used here. To assure it's a boolean, you could use Boolean(this.state.eventData.id) || Boolean(this.state.eventData.sid) or Boolean(this.state.eventData.id || this.state.eventData.sid).
There was a problem hiding this comment.
Yeah, it should be fine.
Description
Fixed loading and editing recurring series and single recurrence data. Full testing unfortunately blocked by olin-build/ABE#210. Also fixed some indentation issues.
Required
Changes must conform to these requirements:
yarn testpasses. All new and existing tests pass.yarn lintpasses. All new code follows the code style of this project.Aspirational
We don't yet require these, but they are nice to have: