Flow type components #235
Flow type components #235
Conversation
Uses appropriate bootstrap components for x button. Changes icon from search -> refresh because a user was having trouble figuring out how to view new users.
Conflicts: lib/admin/components/UserList.js
Codecov Report
@@ Coverage Diff @@
## dev #235 +/- ##
========================================
- Coverage 3.6% 3.55% -0.06%
========================================
Files 327 325 -2
Lines 11839 12035 +196
Branches 4202 4297 +95
========================================
+ Hits 427 428 +1
- Misses 9373 9526 +153
- Partials 2039 2081 +42
Continue to review full report at Codecov.
|
| @@ -86,36 +111,39 @@ export default class AffectedServices extends Component { | |||
| } | |||
| } | |||
|
|
|||
| class ServicesHeader extends Component { | |||
| class ServicesHeader extends Component<{entities: Array<any>}> { | |||
There was a problem hiding this comment.
Should this be Array<AlertEntity> instead? Seems like that would then negate the need for typing within each filter function.
lib/common/components/MapModal.js
Outdated
| this.setState({rectangle: e.layer}) | ||
| // To edit this polyline call : polyline.handler.enable() | ||
| console.log('Path created !', polyline) | ||
| // _onCreate (e) { |
There was a problem hiding this comment.
If we still need this even with comments, please indicate why and perhaps create a github issue.
lib/common/user/Auth0Manager.js
Outdated
| @@ -194,6 +194,7 @@ class Auth0Manager { | |||
| // Get profile with access token and return profile and ID token to store. | |||
| return this._getProfileFromToken(accessToken) | |||
| .then(profile => { | |||
| console.log(profile) | |||
lib/editor/actions/active.js
Outdated
| @@ -116,7 +116,7 @@ export function setActiveGtfsEntity ( | |||
| const {locationBeforeTransitions} = getState().routing | |||
| const pathname = locationBeforeTransitions && locationBeforeTransitions.pathname | |||
| if (!locationBeforeTransitions || !pathname || pathname !== url) { | |||
| // console.log('updating url', url, pathname) | |||
| console.log('updating url', url, pathname) | |||
| selectValue, | ||
| style, | ||
| valueArray | ||
| }: any) => { |
|
|
||
| _onContainsChange = (input) => { | ||
| export default class FareRuleSelections extends Component<Props> { | ||
| _onContainsChange = (input: any) => { |
There was a problem hiding this comment.
Please elaborate a bit on the types of these change functions
lib/editor/components/GtfsEditor.js
Outdated
| handleControlPointDrag: () => void, | ||
| handleControlPointDragEnd: () => void, | ||
| handleControlPointDragStart: () => void, | ||
| setActivePatternSegment: () => void, |
There was a problem hiding this comment.
let's alphabetize these. Try using this plugin.
| mapState: MapState, | ||
| feedSource: Feed, | ||
| feedInfo: FeedInfo, | ||
| zoomToTarget: ?number, |
| } | ||
| } | ||
| } | ||
| // if (activeComponent === 'stop') { |
There was a problem hiding this comment.
Please add a comment explaining why this code is being kept.
There was a problem hiding this comment.
I will remove the code entirely. The click to select stop functionality was essentially completely hidden from users and not entirely reliable or necessary.
| <ControlPointsLayer | ||
| activePattern={activePattern} | ||
| activePattern={this.props.activePattern} |
There was a problem hiding this comment.
Please destructure out all these props.
| activeEntity={this.props.activeEntity} | ||
| activePattern={this.props.activePattern} | ||
| activePatternId={this.props.activePatternId} | ||
| activePatternTripCount={this.props.activePatternTripCount} |
| isSelected: boolean, | ||
| lightText: boolean, | ||
| onChange: (any, number, TimetableColumn, number) => void, | ||
| renderTime: boolean, |
| setOffset: number => void, | ||
| showHelpModal: () => void, | ||
| timetable: TimetableState, | ||
| columns: Array<TimetableColumn>, |
| feedSource: Feed, | ||
| fetchTripsForCalendar: (string, Pattern, string) => void, | ||
| offsetRows: ({rowIndexes: Array<any>, offset: number}) => void, | ||
| saveTripsForCalendar: (string, Pattern, string, Array<Trip>) => void, | ||
| setActiveEntity: (string, string, any) => void, | ||
| setActiveCell: ?string => void, |
| @@ -278,15 +303,15 @@ export default class TimetableGrid extends Component { | |||
| } | |||
| } | |||
|
|
|||
| _renderLeftHeaderCell = ({ columnIndex, key, rowIndex, style }) => ( | |||
| _renderLeftHeaderCell = ({ columnIndex, key, rowIndex, style }: { columnIndex: number, key: string, rowIndex: number, style: Style}) => ( | |||
There was a problem hiding this comment.
Let's unwrap these to be more readable
| setOffset: number => void, | ||
| offsetWithDefaults: (boolean) => void, | ||
| toggleDepartureTimes: () => void, | ||
| addNewRow: (?boolean, ?boolean) => void, |
| gtfsEntitySelected: (string, Entity) => void, | ||
| getGtfsEntity: (string, string) => Entity, | ||
| newRowsDisplayed: Array<any> => void, | ||
| pageCount: number, |
| updateFeedSource: (Feed, any) => void, | ||
| saveFeedSource: (any) => void, | ||
| onHover: (?Feed) => void, | ||
| active: boolean, |
There was a problem hiding this comment.
Lots of great work here. I think we should enable this eslint rule: https://github.com/gajus/eslint-plugin-flowtype#eslint-plugin-flowtype-rules-sort-keys That way we can quickly alphabetize all these type definitions.
|
OK, @evansiroky. I've addressed all of your comments. Good to merge? |
Add flow typing to library (nearly) all components. Notable components this avoids are some of the modal components, which need to be refactored altogether because they rely heavily on refs and accept state/props through their
openmethod, and the Settings components, which should see a refactor in #41.This PR raises the flow coverage to a semi-respectable 76%.