Feature schedule #2
Conversation
package.json
Outdated
| "luxon": "1.3.1", | ||
| "moment": "2.22.2", |
There was a problem hiding this comment.
have you considered a https://github.com/date-fns/date-fns?
There was a problem hiding this comment.
actually, why do you need moment while we have luxon (line above) ? :)
There was a problem hiding this comment.
I did not notice luxon )
| "axios": "0.18.0", | ||
| "bootstrap": "4.1.1", | ||
| "classnames": "2.2.6", | ||
| "core-js": "2.5.7", | ||
| "history": "4.7.2", | ||
| "is-url": "1.2.4", |
There was a problem hiding this comment.
do you really need a dependency for this? 😄
There was a problem hiding this comment.
why not? Why not reuse something that already exists?
| @@ -7,7 +7,7 @@ import { EDUCATION_YEARS, EMPTY_FACULTY, UNIVERSITIES } from '../../reference-da | |||
|
|
|||
| function mapStateToProps(state: any) { | |||
There was a problem hiding this comment.
just a recommendation, feel free to ignore:
mapStateToProps doesn't seem to be a complex selector here, but in general I'd suggest move this alongside with connect to separate file
We've had the following structure which worked well for us
SomePage.js – just a component
connect.js – mapStateToProps, mapDispatchToProps, connect
index.js imports both SomePage and connect, exports connected page
But that's probably just a matter of sense
|
|
||
| type InputType = 'text' | 'tel' | 'email' | 'date' | 'datetime-local' | 'select'; | ||
|
|
||
| class ReduxFormInput extends React.PureComponent<React.InputHTMLAttributes<HTMLInputElement> & WrappedFieldProps> { |
There was a problem hiding this comment.
Messing with internal types from react is not a good idea
Our team had an issue with definitions where we were extending internal classes, but react team changed the hierarchy and everything was broken at some point
There was a problem hiding this comment.
This can happen when you raise the version of the React?
| import * as React from 'react'; | ||
| import { Modal, ModalHeader, ModalBody, ModalFooter, Button } from 'reactstrap'; | ||
|
|
||
| type ModalDeleteProps = { |
| this.setState({ stage, isOpenModalStage: !this.state.isOpenModalStage }); | ||
| }; | ||
|
|
||
| toggleModalEvent = ( |
There was a problem hiding this comment.
don't really understand what is it and how does it work
| } | ||
| } | ||
|
|
||
| export default Schedule; |
There was a problem hiding this comment.
Component is heavy and could be splited into smaller pieces
| } from '../api'; | ||
|
|
||
| export function fetchEventsAndStages(courseId: string) { | ||
| return async (dispatch: any) => { |
There was a problem hiding this comment.
consider using some middleware for async stuff
|
|
||
| import { IEvent, IEventDocument, IStageDocument, IStage } from '../models'; | ||
|
|
||
| type EventsResponse = { |
There was a problem hiding this comment.
what the motivation behind a decision to use types instead of interfaces?
There was a problem hiding this comment.
my guess: here type is about specific thing which doesn't require extension and sharing with other types.
| stages: IStageDocument[], | ||
| ): NormalizeScheduleData[] => { | ||
| const sortedEvents = events | ||
| .reduce<INormalizeEvent[]>((res, event) => { |
There was a problem hiding this comment.
you don't really need pass a type to reduce, ts understands return value of reduce
API