Merge button for PR screen#60
Conversation
| import { TouchableOpacity, StyleSheet, ImageBackground } from 'react-native'; | ||
|
|
||
| type Props = { | ||
| children: Object, |
| onPress: () => any, | ||
| style?: ComponentStyles | ||
| style?: ComponentStyles, | ||
| textStyle: Object |
| return ( | ||
| <TouchableOpacity onPress={onPress} style={[styles.button, style]}> | ||
| <UIText style={styles.text}> | ||
| <UIText style={textStyle !== {} ? textStyle : styles.text}> |
| export default class ImageButton extends PureComponent<Props> { | ||
| render() { | ||
| const { onPress } = this.props; | ||
| const image = require('../../assets/threeDots.png'); |
There was a problem hiding this comment.
You can use import, but will be better to use vector icons, we already use https://github.com/oblador/react-native-vector-icons
|
|
||
| const styles = StyleSheet.create({ | ||
| imageBackground: { | ||
| width:50, height: 20, marginTop: 10, |
| styleText={styles.buttonText}> | ||
| Close PR | ||
| </ImageButton> | ||
| <Modal |
There was a problem hiding this comment.
Please use openModal instead of manually do modals
| ".js" | ||
| ] | ||
| } | ||
| "name": "ghubber", |
There was a problem hiding this comment.
Please revert ident for this file
Thanks!
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)
| "ThunkAction": true, | ||
| "ActionType": true | ||
| "ActionType": true, | ||
| "require": true |
| ]; | ||
| } | ||
|
|
||
| setModalVisible(visible) { |
|
|
||
| class RepositoryPullRequestScreen extends PureComponent<Props> { | ||
| class RepositoryPullRequestScreen extends PureComponent<Props, ModalState> { | ||
| state: ModalState = { |
| export default connect( | ||
| null, | ||
| { addModal } | ||
| )(ImageButton); |
There was a problem hiding this comment.
connecting a component to the redux store w/o props passing is the wrong way.
Bind dispatch to addModal where you're rendering this component and pass it as a prop here. But seems like you don't use it here, so remove connecting at all.
| @@ -0,0 +1,2 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please, remove this unnecessary empty line
| <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.
Related reading
| 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.
3 participants
I've made UI for now, and later I can configure/add redux and api actions for merging.