[$16,000] Let's fix a React Native issue -- useWindowDimensions() returns swapped height and width in iOS #2727
Comments
|
This issue has not been updated in over 15 days. eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
|
Removed the monthly label so it won't auto-close. Also dropped a (likely fruitless) post in #expensify-open-source to see if anyone has ideas. https://expensify.slack.com/archives/C01GTK53T8Q/p1633992309160300 |
|
@deetergp do we know that they are actively working on a fix? If not, perhaps we can hire a contributor to submit a PR to fix it to RN directly now. |
Your guess is as good as mine… The last comment on that issue was from a year ago. There are references from other issues and PRs as recently as the end of October '21 so it is definitely still in people's minds. 🤷 |
|
@deetergp, this Monthly task hasn't been acted upon in 6 weeks; closing. If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead. |
|
I still think we should hire a contributor to fix this PR directly in RN facebook/react-native#29290. I'm going to add the external label for it. |
puneetlath
added
the
External
|
Triggered auto assignment to @NicMendonca ( |
NicMendonca
changed the title
|
Upwork job: https://www.upwork.com/jobs/~017b165dfa9821e51c |
|
@parasharrajat , can you please accept so I can pay you and close this out? |
mallenexpensify
removed
the
External
mallenexpensify
added
the
External
|
Triggered auto assignment to @joekaufmanexpensify ( |
|
Current assignee @parasharrajat is eligible for the External assigner, not assigning anyone new. |
melvin-bot
bot
added
the
Help Wanted
|
Current assignee @iwiznia is eligible for the External assigner, not assigning anyone new. |
melvin-bot
bot
changed the title
|
@parasharrajat please accept the job and @joekaufmanexpensify will pay you out (I'm heading OOO) https://www.upwork.com/jobs/~01789a28c4422d1d15 |
|
I will accept it in 2-3 days. |
|
Just assigned this issue, and it seems like all that needs to be done here is pay @parasharrajat as soon as they accept the job. |
|
Job still needs to be accepted before this can be paid. |
joekaufmanexpensify
changed the title
|
Same. |
|
@parasharrajat Any update on accepting this so we can pay you? |
|
@joekaufmanexpensify Sent a proposal. |
|
Great, thanks! Hired. |
|
@parasharrajat payment sent and contract ended. |
|
Closed job in upwork. Closing as payment has been issued, and this is all set! |
facebook#34014) Summary: This fix solves a problem very well evaluated [here](Expensify/App#2727) as well as this [one](facebook#29290). The issue is that when the app goes to background, in landscape mode, the RCTDeviceInfo code triggers an orientation change event that did not physically happen. Due to that, we get swapped values returned when going back to the app. I debugged the react-native code, and to me it seems that react native publishes the orientation change event one extra time when switching the state of the app to 'inactive'. Here is what is happening: 1. iPad is in landscape. 2. We move the app to inactive state. 3. Native Code queues portrait orientation change (no such change happened physically), and immediately after it triggers landscape change (same as we had in point 1). 4. We restore the app to active state. 5. The app receives two queued orientation change events, one after another. 6. The quick transition between portrait and landscape happens even though it never went back to portrait. Fresh `react-native init` app repro case can be found here: https://github.com/lbaldy/issue-34014-repro Video presenting the issue (recorded while working on: Expensify/App#2727 ): https://www.youtube.com/watch?v=nFDOml9M8w4 <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [iOS] [Fixed] - Fix the way the orientation events are published, to avoid false publish on orientation change when app changes state to inactive Pull Request resolved: facebook#34014 Test Plan: 1. Make sure you have a working version of E/App. 2. Open App/src/components/withWindowDimensions.js and update the constructor by changing this line: `this.onDimensionChange = _.debounce(this.onDimensionChange.bind(this), 100);` to `this.onDimensionChange = this.onDimensionChange.bind(this);` 3. Open the NewExpensify.xcodeproj in xCode. 4. Open the RCTDeviceInfo.mm file and replace it's contents with the file from this PR. 5. Select your device of choice (I suggest starting with iPad mini) and run the app though xCode. 6. From this point you can move to the test scenarios described below. Reproduction + Fix test video (Test 1): https://youtu.be/jyzoNHLYHPo Reproduction + Fix test video (Test 2): https://youtu.be/CLimE-Fba-g **Test 1:** 1. Launch app in portrait, open chat - no sidebar visible. 7. Switch to landscape - sidebar shows. 8. Put app to background. 9. Put app back to foreground - make sure the side menu doesn't flicker. **Test 2:** 1. Launch app in portrait, open chat - no sidebar visible. 2. Switch to landscape - sidebar shows. 3. Put app to background. Switch orientation back to portrait. 4. Put app back to foreground - make sure the side menu hides again as it should be in portrait. Reproduction + Fix test video (Test 3, Test 4): https://youtu.be/EJkUUQCiLRg iPad mini test 1 applies. Scenario 2 does not as the screen is too wide in both orientations and iPad pro shows sidebar always. **Test 3:** 1. launch the app. 2. Make sure you're in landscape mode. 3. See split screen with some other app. Make sure the side bar is visible. 4. Play with the size of the view, resize it a bit. When the view shrinks it should hide the sidebar, when it grows it should show it. 10. Move the app to background and back to foreground, please observe there are no flickers. **Test 4:** 1. Launch the app. 2. Make sure you're in landscape mode. 3. Make the multitasking view and make Expensify app a slide over app. 4. Move back to fullscreen/split screen. Make sure the menu is shown accordingly 5. Move the app to background and back to foreground, please observe there are no flickers. Non reg with and without the fix video: https://youtu.be/kuv9in8vtbk Please perform standard smoke tests on transformation changes. Reviewed By: cipolleschi Differential Revision: D37239891 Pulled By: jacdebug fbshipit-source-id: e6090153820e921dcfb0d823e0377abd25225bdf
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
We have an issue where opening the app in native iOS on an iPad in Landscape orientation, the Chat Selector component is floating in the middle of the screen. We opened this issue to deal with it and had a workaround put forward by one of our contributors, but in the process of investigating, discovered that the real issue is with React Native core. They have an open issue and are working on a fix, but it's anyone's guess when it will be ready.
Let's keep an eye on the React Native team's solution and see if we can put it into place and undo the the temporary workaround we are using for now.
cc @marcaaron @mallenexpensify
Workaround:
#2180 (comment)
Original React Native Issue
facebook/react-native#29290
Platform:
Where is this issue occurring?
Version Number: 1.0.2-51
Notes/Photos/Videos: See the original issue
View all open jobs on Upwork
The text was updated successfully, but these errors were encountered: