update layout on orientation change #2
Conversation
index.js
Outdated
| import PageData from './components/PageData'; | ||
| import Paginator from './components/Paginator'; | ||
|
|
||
| var {height, width} = Dimensions.get('window'); |
There was a problem hiding this comment.
Can you change this to const and style the curlies as { height, width } please?
index.js
Outdated
| this.state = { | ||
| currentPage: 0, | ||
| layout:{ | ||
| height:height, |
index.js
Outdated
| }; | ||
|
|
||
| updatePosition = (event) => { | ||
| const { contentOffset, layoutMeasurement } = event.nativeEvent; |
There was a problem hiding this comment.
Seems like layoutMeasurement is no longer needed
|
Thanks for your PR! Please address the minor code styling issues I've pointed out |
|
I committed a change with your suggestions |
|
Did you see my code update ? |
|
Nice! Sorry, busy with my other projects at the moment. I run the code in a simulator this weekend to see if it works properly. |
goshacmd
left a comment
goshacmd
left a comment
There was a problem hiding this comment.
Hey, thanks for contributing! Did you run this on iOS and/or Android?
Also, can you please fix these three minor code style issues and squash the commits? Looks good otherwise!
| import PageData from './components/PageData'; | ||
| import Paginator from './components/Paginator'; | ||
|
|
||
| var { height, width } = Dimensions.get('window'); |
There was a problem hiding this comment.
Minor nitpick: can you switch to const please?
|
|
||
| this.state = { | ||
| currentPage: 0, | ||
| layout:{ |
There was a problem hiding this comment.
Styling nitpick: please add a space after the colon :)
| currentPage: 0, | ||
| layout:{ | ||
| height: height, | ||
| width: width, |
There was a problem hiding this comment.
Both of these can use the ES6 shorthand syntax:
layout: {
height,
width,
}
|
@acerbetti can you update the styling issues so we can move the PR forward? |
2 participants
No description provided.