This repository has been archived by the owner on Mar 9, 2022. It is now read-only.
Ca 37 - First iteration of Contributors List view, fully functional. #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… by join data for now, will also sort by inactive and style conditionally
…, inactive contrib thumbnail at 50% opacity
…ctioning, needs styles and devTeamIcon Pedal; first iteration
rabbitwerks
requested review from
thetornadotitan,
w3cj,
AndCotOli,
ewenjo and
spiray
|
Thanks @rabbitwerks, Can you resolve the merge conflict before I start the review? |
spiray
reviewed
Jun 11, 2019
| import { View, Text } from 'native-base'; | ||
| import { View } from 'native-base'; | ||
| import PageHeader from './pageHeader'; | ||
| import ListContainer from './listContainer'; | ||
|
|
||
| const styles = StyleSheet.create({ |
There was a problem hiding this comment.
Can this be destructured to
const { content } = StyleSheet.create...| } | ||
|
|
||
| componentDidMount() { | ||
| fetch('https://api-dev.coding.garden/contributors') |
There was a problem hiding this comment.
We decided to use async/await for all promises on the backend and enforced it with an eslint plugin. I think we should follow that on frontend as well.
| fetch('https://api-dev.coding.garden/contributors') | ||
| .then(res => res.json()) | ||
| .then((data) => { | ||
| // sortData(data); |
There was a problem hiding this comment.
If we are sure that this isn't being used it should be removed.
| .then(res => res.json()) | ||
| .then((data) => { | ||
| // sortData(data); | ||
| console.log(data); |
There was a problem hiding this comment.
No lingering console logs should remain in the code.
| dataLoaded: true | ||
| }); | ||
| }) | ||
| .catch(error => console.log(error)); |
There was a problem hiding this comment.
Should state be explicitly set here as part of the error handling?
| countryCode, | ||
| active | ||
| } = this.props.contributorData; | ||
| const { devTeams } = this.props; |
There was a problem hiding this comment.
This can be merged with the index destructure on line 62.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first iteration of the Contributors List view.
This feature will need additional iterations in functionality and design. Profile Icon is a placeholder, no functionality. Each contrib item has a toggle-able more info section. The ovals representing the Dev Teams for each contributor will be replaced with the proper petal icon and color coding.
Right now each main cmp view has a dedicated BG Color to help show the layout of the cmps.
All of this will be refactored into a nicer looking view. The page header will need to be refactored out to a reusable cmp.