Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea for orientation change support #9 #21

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

abdallahm
Copy link

This is an idea of supporting the orientation change, however it depends on another package for detecting the orientation change event, like react-native-orientation.

Please review the changes and let me know if you have any questions or concerns.

@abdallahm
Copy link
Author

i think the checks failed because i had to insert the sheet source into the styles object to be able to retrieve the original source and reprocess it again.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.003%) to 96.717% when pulling 1f49a26 on abdallahm:master into ccea7a3 on vitalets:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 99.242% when pulling 83a7f0c on abdallahm:master into ccea7a3 on vitalets:master.

@vitalets
Copy link
Owner

vitalets commented Aug 9, 2016

Hi @abdallahm

Could you make some effort in direction when we wrap the whole app in our <AppContainer> component and listen it's onLayout event to detect orientation change?
In that case we avoid additional orientation module.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 99.746% when pulling 32cd544 on abdallahm:master into ccea7a3 on vitalets:master.

@abdallahm
Copy link
Author

Hi @vitalets

Done, please check.

@vitalets
Copy link
Owner

vitalets commented Aug 12, 2016

I've pushed my drafts on this feature to separate branch orientation-support, lets move our work there.

I suggest creating layout module that will do all job for processing layout changes (not in utils as in your pr)
Here it is in more or less ready state: https://github.com/vitalets/react-native-extended-stylesheet/blob/orientation-support/src/layout.js

Also I suggest to provide <AppContainer> component. Those developers who want to listen orientation change, should just wrap application into it.

AppContainer itself: https://github.com/vitalets/react-native-extended-stylesheet/blob/orientation-support/src/components/app-container.js

Example of wrapping app:
https://github.com/vitalets/react-native-extended-stylesheet/blob/orientation-support/examples/orientation-change/app.js

The problem is to automatically update react components after orientation change.
For that my thought was to register each component in tracker that will push updates after orientation change:
https://github.com/vitalets/react-native-extended-stylesheet/blob/orientation-support/src/components/tracker.js
This approach was not finally ready. So it woulb be great if play with stuff in orientation-change branch and make PR if got it working..

@vdhpieter
Copy link

Is there any progress on this? It would be very awesome!

@vitalets
Copy link
Owner

vitalets commented Nov 8, 2016

hi @vdhpieter

I am currently a bit out of mobile unfortunately..

@Antoine-C
Copy link

Hi !

Any update on this feature?

@vitalets
Copy link
Owner

Hi @Antoine-C

unfortunately not :(

@stale
Copy link

stale bot commented Dec 12, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 12, 2018
@vitalets vitalets added pinned and removed wontfix labels Dec 12, 2018
}
}
return window.dimensions = {
orientation: (layout.width < layout.height ? 'portrait' : 'landscape'),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this can be a false positive on iPad when using split view after this bug was fixed:
facebook/react-native#16152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants