Conversation
…tive-poc into paginated-list
| import variable from './../variables/platform'; | ||
|
|
||
| export default (variables /*: * */ = variable) => { | ||
| const bodyTheme = { |
There was a problem hiding this comment.
directly return from here, no need to create variable.
There was a problem hiding this comment.
Above code is in Native base theme third party Library.
| @@ -0,0 +1,90 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
we can use file naming conventions for such fil as Form.style.js and for component Form.js
There was a problem hiding this comment.
Above code is in Native base theme third party Library.
EXPO/screens/Constants.js
Outdated
| @@ -0,0 +1,25 @@ | |||
| var localDB ={ | |||
There was a problem hiding this comment.
should be export localDB
EXPO/screens/DrawerScreen.js
Outdated
| </Text> | ||
| </View> | ||
| <View style={styles.menuItem}> | ||
| <Image source={{uri:'https://img.icons8.com/ios/60/000000/google-nearby.png'}}/> |
There was a problem hiding this comment.
uri should be constant
There was a problem hiding this comment.
Changes has been done as per suggestions.
|
|
||
| export default DrawerScreen; | ||
|
|
||
| const styles = StyleSheet.create({ |
There was a problem hiding this comment.
we can move this to separate file DrawerScreen.style.js and export from there.
I recommend to create directory with name DrawerScreen under that all the files related to that will exist.
eg. DrawerScreen.jsx(component), DrawerScreen.test.js(test cases) & DrawerScreen.style.js(stylesheets)
EXPO/screens/GridScreen.js
Outdated
| @@ -0,0 +1,116 @@ | |||
| import React from "react"; | |||
| import { FlatList, StyleSheet, Text, View, Image } from "react-native"; | |||
| import { Button } from "react-native-elements"; | |||
There was a problem hiding this comment.
combine 2 imports
EXPO/screens/GridScreen.js
Outdated
| }; | ||
|
|
||
| componentWillMount() { | ||
| this.getUserInfo(0); |
There was a problem hiding this comment.
this should be in componentDidMount, componentWillMount is deprecated in higher version of react. and we should pass this.state.page to method getUserInfo as its initial value is 0
EXPO/screens/GridScreen.js
Outdated
| getUserInfo(page) { | ||
| console.log("getting user info fior page " + page); | ||
|
|
||
| const URL = `https://reqres.in/api/users/?page=${page}`; |
There was a problem hiding this comment.
should be: const URL = https://reqres.in/api/users/?page=${this.state.page};
EXPO/screens/GridScreen.js
Outdated
| ); | ||
|
|
||
| gotoNextPage = () => { | ||
| this.getUserInfo(this.state.page + 1); |
There was a problem hiding this comment.
should be:
this.setState({
page: this.state.page < this.state.total_pages ? this.state.page + 1 : this.state.page,
}, this.getUserInfo);
EXPO/screens/GridScreen.js
Outdated
| }; | ||
|
|
||
| gotoPreviousPage = () => { | ||
| this.getUserInfo(this.state.page - 1); |
There was a problem hiding this comment.
should be:
this.setState({
page: this.state.page > 1 ? this.state.page - 1 : this.state.page,
}, this.getUserInfo);
There was a problem hiding this comment.
We are not using Gridscreen.js. So file has been deleted
EXPO/screens/GridScreen.js
Outdated
| this.getUserInfo(0); | ||
| } | ||
|
|
||
| getUserInfo(page) { |
There was a problem hiding this comment.
no need to pass page as parameter as we can refer it as state variable
There was a problem hiding this comment.
We are not using Gridscreen.js. So file has been deleted
EXPO/screens/HomeScreen.js
Outdated
| @@ -1,188 +1,676 @@ | |||
| import React from 'react'; | |||
| /*import React from "react"; | |||
There was a problem hiding this comment.
clean the commented code
There was a problem hiding this comment.
Changes has been done as per suggestions.
EXPO/screens/HomeScreen.js
Outdated
| </View> | ||
| ); | ||
|
|
||
| mapStyle = [ |
There was a problem hiding this comment.
move to constants or config file
EXPO/screens/HomeScreen.js
Outdated
| } | ||
| ]; | ||
| componentDidMount() { | ||
| Geocoder.init("AIzaSyD7JZmztK5wE-80P8t-_IOHZQinVtx4Dio"); |
There was a problem hiding this comment.
token should be from config
EXPO/screens/HomeScreen.js
Outdated
| ScreenOrientation.allowAsync(ScreenOrientation.Orientation.ALL); | ||
| if (Platform.OS === "android" && !Constants.isDevice) { | ||
| console.log("Permission denied"); | ||
| this.setState({ |
There was a problem hiding this comment.
we can not do setState on component which is not mounted yet, use componentDidMount instead
EXPO/screens/HomeScreen.js
Outdated
| } | ||
| } | ||
|
|
||
| const styles = StyleSheet.create({ |
There was a problem hiding this comment.
move to component style file
EXPO/screens/MapScreen.js
Outdated
| } | ||
| }; | ||
|
|
||
| mapStyle = [ |
There was a problem hiding this comment.
move to separate constant or config file
EXPO/screens/MapScreen.js
Outdated
| console.log("getLatsstLongs called "+locationName); | ||
| Geocoder.from(locationName) | ||
| .then(json => { | ||
| var location = json.results[0].geometry.location; |
There was a problem hiding this comment.
use try/catch for better error handling, what if json.results is undefined OR json.results having length 0
EXPO/screens/MapScreen.js
Outdated
| }); | ||
| } | ||
|
|
||
| componentWillMount() { |
There was a problem hiding this comment.
use componentDidMount, we can not do setState before component mount
There was a problem hiding this comment.
Changes has been done as per suggestions.
EXPO/screens/MapScreen.js
Outdated
| } | ||
|
|
||
|
|
||
| _getLocationAsync = async () => { |
There was a problem hiding this comment.
repetitive code, we can create utility async function for this
EXPO/screens/UserInfoDBManager.js
Outdated
| export default class UserInfoDBManager extends Component { | ||
|
|
||
| constructor() { | ||
| this.sqlite = SQLite; |
There was a problem hiding this comment.
UserInfoDBManager.js file has been deleted.No used in App
EXPO/screens/UserInfoDBManager.js
Outdated
| location: "1" | ||
| }).then((db) => { | ||
| this.dbInstance = db; | ||
| }) |
There was a problem hiding this comment.
error case not handled ?
EXPO/screens/UserInfoDBManager.js
Outdated
|
|
||
| createTable() { | ||
| return new Promise((resolve, reject) => { | ||
| this.dbInstance.executeSql('CREATE TABLE IF NOT EXISTS ' + localDB.tableName.tblLogin + ' (id INTEGER PRIMARY KEY ,firstName TEXT,lastName TEXT,avator:TEXT)' |
There was a problem hiding this comment.
what is this.dbInstance is undefined ? this is the case where DB connection failed.
EXPO/screens/UserInfoDBManager.js
Outdated
| this.dbInstance.executeSql( | ||
| "DELETE FROM"+ localDB.tableName.tblLogin | ||
| ).then((val) => { | ||
| resolve(true); |
There was a problem hiding this comment.
no need to return true/false explicitly, you can tread resolve callback for positive flow and reject callback for error flow
…dded, 4.Gallary functionality added
- Code segregation - code optimized
4 participants
No description provided.