This repository has been archived by the owner on Sep 24, 2020. It is now read-only.
Merge button for PR screen #60
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
|
|
||
| import React, { PureComponent } from 'react'; | ||
| import { TouchableOpacity, StyleSheet } from 'react-native'; | ||
| import Icon from 'react-native-vector-icons/FontAwesome'; | ||
| import { connect } from 'react-redux'; | ||
| import { addModal } from 'actions'; | ||
|
|
||
| type Props = { | ||
| children: React$Element, | ||
| onPress: () => any, | ||
| style?: ComponentStyles, | ||
| addModal: typeof addModal | ||
| }; | ||
|
|
||
| class ImageButton extends PureComponent<Props> { | ||
| render() { | ||
| const { onPress } = this.props; | ||
| const image = 'ellipsis-h'; | ||
| return ( | ||
| <TouchableOpacity | ||
| onPress={onPress} | ||
| style={styles.imageBackground}> | ||
| <Icon name={image} size={20} /> | ||
| </TouchableOpacity> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export default connect( | ||
| null, | ||
| { addModal } | ||
| )(ImageButton); | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| imageBackground: { | ||
| width:50, | ||
| height: 20, | ||
| marginTop: 10, | ||
| borderColor: '#edf1f5', | ||
| borderWidth: 1, | ||
| backgroundColor: '#edf1f5', | ||
| justifyContent: 'center', | ||
| alignItems: 'center' | ||
| } | ||
| }); | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
|
|
||
|
There was a problem hiding this comment. Please, remove this unnecessary empty line |
||
| export { default as ImageButton } from './ImageButton'; | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,10 @@ | |
|
|
||
| import React, { PureComponent } from 'react'; | ||
| import { connect } from 'react-redux'; | ||
| import { View, StyleSheet, ScrollView, FlatList, Text, TouchableOpacity } from 'react-native'; | ||
| import { ErrorView, Spinner, Badge, UIText, ReactionGroup, Blank, Comment, CommitRow, RowSeparator, ImageButton, ModalPicker, Button } from 'components'; | ||
| import { IndicatorViewPager, PagerTitleIndicator } from 'rn-viewpager'; | ||
| import { fetchPullRequest, showRepositoryCommit, addModal, closeModal } from 'actions'; | ||
| import { normalizeFont } from 'utils/helpers'; | ||
|
|
||
| // import flow types | ||
|
|
@@ -23,6 +23,12 @@ type Props = { | |
| }, | ||
| fetchPullRequest: typeof fetchPullRequest, | ||
| showRepositoryCommit: typeof showRepositoryCommit, | ||
| addModal: typeof addModal, | ||
| closeModal: typeof closeModal | ||
| } | ||
|
|
||
| type ModalState = { | ||
| modalVisible: boolean, | ||
| } | ||
|
|
||
| const TITLE_COMMITS_INDEX = 1; | ||
|
|
@@ -40,7 +46,11 @@ function getStateColor(state: string): string { | |
| } | ||
| } | ||
|
|
||
| class RepositoryPullRequestScreen extends PureComponent<Props, ModalState> { | ||
| state: ModalState = { | ||
|
|
||
| modalVisible: false | ||
| }; | ||
|
|
||
| componentWillMount() { | ||
| this.fetchPullRequest(); | ||
| } | ||
|
|
@@ -88,6 +98,10 @@ class RepositoryPullRequestScreen extends PureComponent<Props> { | |
| ]; | ||
| } | ||
|
|
||
| setModalVisible(visible) { | ||
|
|
||
| this.setState({ modalVisible: visible }); | ||
| } | ||
|
|
||
| renderOverView(pullRequest: Object): React.Element<any> { | ||
| return ( | ||
| <ScrollView style={styles.overview}> | ||
|
|
@@ -101,6 +115,36 @@ class RepositoryPullRequestScreen extends PureComponent<Props> { | |
| </View> | ||
| </View> | ||
| <UIText style={styles.body}>{pullRequest.body}</UIText> | ||
| <ImageButton | ||
| style={styles.button} | ||
| onPress={() => this.props.addModal( | ||
|
There was a problem hiding this comment. Such kind of function passing is the anti-pattern. |
||
| <ModalPicker | ||
| data={[{ type: 'merge' }]} | ||
| renderOption={ | ||
| ({ item }) => { | ||
| return ( | ||
|
There was a problem hiding this comment. You can omit it. renderOption={({ item }) => (
<TouchableOpacity
key={item.type}
style={styles.modalContent}
onPress={() => { // <- remove this arrow function passing
closeModal();
}}
>
<Text style={styles.modalTitle}>Merge this pull request?</Text>
<Button
style={styles.deletePr}
textStyle={styles.deletePrText}
onPress={() => this.props.closeModal()} // <- remove this arrow function passing
>
Merge
</Button>
</TouchableOpacity>
)}But, I do prefer extract this rendering to a separate function. |
||
| <TouchableOpacity | ||
| key={item.type} | ||
| style={styles.modalContent} | ||
| onPress={() => { | ||
| closeModal(); | ||
| }} | ||
| > | ||
| <Text style={styles.modalTitle}>Merge this pull request?</Text> | ||
| <Button | ||
| style={styles.deletePr} | ||
| textStyle={styles.deletePrText} | ||
| onPress={() => this.props.closeModal()}> | ||
| Merge | ||
| </Button> | ||
| </TouchableOpacity> | ||
| ); | ||
| } | ||
| } | ||
| /> | ||
| )} | ||
| styleText={styles.buttonText}> | ||
| </ImageButton> | ||
| <ReactionGroup reactions={pullRequest.reactionGroups} /> | ||
| <FlatList | ||
| style={styles.commentsList} | ||
|
|
@@ -216,6 +260,40 @@ const styles = StyleSheet.create({ | |
| flex: 1, | ||
| paddingHorizontal: 10 | ||
| }, | ||
| button: { | ||
| width: 80, height: 30, | ||
| backgroundColor: '#edf1f5', | ||
| }, | ||
| deletePr: { | ||
| width: 70, marginTop: 10, | ||
| backgroundColor: '#52c245', | ||
| }, | ||
| deletePrText: { | ||
| color: '#fff', | ||
| fontSize: 14, | ||
| fontWeight: 'bold', | ||
| textAlign: 'center', | ||
| }, | ||
| buttonText: { | ||
| color: 'black', | ||
| fontSize: 14, | ||
| fontWeight: '600', | ||
| textAlign: 'center', | ||
| backgroundColor: 'transparent' | ||
| }, | ||
| modalTitle: { | ||
| color: 'black', | ||
| fontSize: 14, marginTop: 8, | ||
| fontWeight: '600', | ||
| textAlign: 'center', | ||
| }, | ||
| modalContent: { | ||
| flex:1, borderRadius: 10, | ||
| backgroundColor: '#fff', | ||
| flexDirection: 'column', | ||
| alignItems: 'center', | ||
| marginBottom: 10 | ||
| }, | ||
| wrapper: { | ||
| flex: 1 | ||
| }, | ||
|
|
@@ -263,5 +341,5 @@ export default connect( | |
| state: state.repositoryPullRequest | ||
| }; | ||
| }, | ||
| { fetchPullRequest, showRepositoryCommit, addModal, closeModal } | ||
| )(RepositoryPullRequestScreen); | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,80 +1,80 @@ | ||
| { | ||
| "name": "ghubber", | ||
|
There was a problem hiding this comment. Please revert ident for this file There was a problem hiding this comment. I'm not sure what to revert here? There was a problem hiding this comment. I mean, not needed to change this file (code indentation - spaces) |
||
| "version": "0.9.0", | ||
| "private": true, | ||
| "scripts": { | ||
| "start": "node node_modules/react-native/local-cli/cli.js start --reset-cache", | ||
| "start:ios": "concurrently -r 'npm run start' 'npm run ios && google-chrome http://localhost:8081/debugger-ui'", | ||
| "start:android": "concurrently -r 'npm run start' 'npm run android && google-chrome http://localhost:8081/debugger-ui'", | ||
| "ios": "node node_modules/react-native/local-cli/cli.js run-ios", | ||
| "android": "node node_modules/react-native/local-cli/cli.js run-android", | ||
| "flow": "flow check", | ||
| "test": "jest", | ||
| "lint": "eslint --cache .", | ||
| "lint:staged": "staged-files '**/*.js' -- $(npm bin)/eslint", | ||
| "postversion": "react-native-version" | ||
| }, | ||
| "pre-commit": { | ||
| "run": "lint:staged" | ||
| }, | ||
| "dependencies": { | ||
| "babel-plugin-module-resolver": "^2.7.1", | ||
| "babel-plugin-transform-define": "^1.3.0", | ||
| "babel-plugin-transform-remove-console": "^6.8.5", | ||
| "base-64": "^0.1.0", | ||
| "commonmark": "^0.28.1", | ||
| "commonmark-react-renderer": "^4.3.3", | ||
| "github-flow-js": "^0.18.2", | ||
| "i18n-js": "^3.0.1", | ||
| "lodash": "^4.17.4", | ||
| "moment": "^2.18.1", | ||
| "query-string": "^5.0.0", | ||
| "react": "16.0.0-alpha.12", | ||
| "react-native": "^0.48.4", | ||
| "react-native-device-info": "^0.11.0", | ||
| "react-native-htmlview": "^0.12.0", | ||
| "react-native-sentry": "^0.25.0", | ||
| "react-native-side-menu": "^1.1.3", | ||
| "react-native-swiper": "^1.5.12", | ||
| "react-native-vector-icons": "^4.4.0", | ||
| "react-navigation": "^1.0.0-beta.12", | ||
| "react-redux": "^5.0.6", | ||
| "redux": "^3.7.2", | ||
| "redux-logger": "^3.0.6", | ||
| "redux-thunk": "^2.2.0", | ||
| "rn-viewpager": "^1.2.4" | ||
| }, | ||
| "devDependencies": { | ||
| "babel-eslint": "^8.0.1", | ||
| "babel-jest": "21.2.0", | ||
| "babel-preset-react-native": "4.0.0", | ||
| "concurrently": "^3.5.0", | ||
| "eslint": "^4.8.0", | ||
| "eslint-plugin-flowtype": "^2.37.0", | ||
| "eslint-plugin-react": "^7.4.0", | ||
| "eslint-plugin-react-native": "^3.1.0", | ||
| "flow-bin": "^0.56.0", | ||
| "jest": "21.2.1", | ||
| "pre-commit": "^1.2.2", | ||
| "react-native-version": "^2.3.1", | ||
| "react-test-renderer": "16.0.0-alpha.12", | ||
| "sentry-cli-binary": "^1.20.0", | ||
| "staged-files": "^0.1.2" | ||
| }, | ||
| "jest": { | ||
| "preset": "react-native" | ||
| }, | ||
| "contributes": { | ||
| "languages": [ | ||
| { | ||
| "id": "flow", | ||
| "aliases": [ | ||
| "Flow" | ||
| ], | ||
| "extensions": [ | ||
| ".js" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connecting a component to the redux store w/o props passing is the wrong way.Bind
dispatchtoaddModalwhere you're rendering this component and pass it as a prop here. But seems like you don't use it here, so removeconnecting at all.