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

Dimensions window support for Split View and Slide Over on iPad #16152

Open
Freddy03h opened this Issue Sep 30, 2017 · 17 comments

Comments

Projects
None yet
@Freddy03h
Contributor

Freddy03h commented Sep 30, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 7.2.1
Yarn: 0.24.6
npm: 3.10.10
Watchman: 4.7.0
Xcode: Xcode 9.0 Build version 9A235
Android Studio: 2.3 AI-162.4069837

Packages: (wanted => installed)
react: 16.0.0-alpha.12 => 16.0.0-alpha.12
react-native: 0.48.4 => 0.48.4

Steps to Reproduce

On on iPad, with the app on "Split View" or "Slide Over".

console.log(Dimensions.get('screen')) // {fontScale: 1, width: 768, height: 1024, scale: 2}
console.log(Dimensions.get('window')) // {fontScale: 1, width: 768, height: 1024, scale: 2}

(example of console log on iPad Pro 9.7 inch)

Expected Behavior

On iOS Dimensions.get('screen') and Dimensions.get('window') always return the same data. It assuming that the app is always in fullscreen. But on iPad the screen can be share with an other app (Split View), and since iOS, can "float" in a little window over an other app (Slide Over).

Dimensions.get('window') shouldn't always return Dimensions.get('screen') width and height but the real size of the window.

Actual Behavior

Dimensions.get('window') always return Dimensions.get('screen'), even on "Split View" or "Slide Over"

Slide Over example

capture d ecran 2017-10-01 a 01 32 22

console.log(Dimensions.get('window')) // {fontScale: 1, width: 768, height: 1024, scale: 2}

Split View example 1

capture d ecran 2017-10-01 a 01 32 31

console.log(Dimensions.get('window')) // {fontScale: 1, width: 768, height: 1024, scale: 2}

Split View example 2

capture d ecran 2017-10-01 a 01 33 00

console.log(Dimensions.get('window')) // {fontScale: 1, width: 768, height: 1024, scale: 2}

Reproducible Demo

It can be easily tested on iPad by adding console.log(Dimensions.get('window')) anywhere in an existing app or in a fresh create-react-native-app.
And using Split View and Slide Over features of iOS on iPad.

Snack Expo is here : https://snack.expo.io/ryIHToaiW but it need to be tested on an iPad Device or Simulator.

@Freddy03h

This comment has been minimized.

Contributor

Freddy03h commented Oct 2, 2017

I find a way to send correct width and height for the window.
In RCTDeviceInfo.m I change RCTExportedDimensions :

static NSDictionary *RCTExportedDimensions(RCTBridge *bridge)
{
  RCTAssertMainQueue();

  // Don't use RCTScreenSize since it the interface orientation doesn't apply to it
  CGRect screenSize = [[UIScreen mainScreen] bounds];
  NSDictionary *dimsScreen = @{
                         @"width": @(screenSize.size.width),
                         @"height": @(screenSize.size.height),
                         @"scale": @(RCTScreenScale()),
                         @"fontScale": @(bridge.accessibilityManager.multiplier)
                         };
  CGRect windowSize = [RCTKeyWindow() bounds];
  NSDictionary *dimsWindow = @{
                         @"width": @(windowSize.size.width),
                         @"height": @(windowSize.size.height),
                         @"scale": @(RCTScreenScale()),
                         @"fontScale": @(bridge.accessibilityManager.multiplier)
                         };

  return @{
           @"window": dimsWindow,
           @"screen": dimsScreen
           };
}

It send the correct value but it didn't trigger a change event on Dimensions when resizing the app window in a Split-View mode.

Currently, two fonctions in Obj-C can trigger this event : didReceiveNewContentSizeMultiplier and interfaceOrientationDidChange. I think we need to create a new observer for the event, but I'm not an iOS dev, and I don't understand all the code in this file. Also, I don't get why it use NotificationCenter stuffs or why they use the status bar orientation to get the orientation change of all the screen…

edit: ok apparently NotificationCenter is just a EventEmmiter, nothing related to Notifications for end-user (I thought that it was a very tricky code ^^')

Freddy03h referenced this issue Oct 6, 2017

Separate window dimensions change from orientation
Summary:
**Summary:**
There was a bug with RN.Dimensions returning incorrect window dimensions. In certain cases when device was in portrait, window dimensions reported landscape dimensions and vice versa.

This happened because in certain scenarios, after device orientation changed, dimensions update event from ReactRootView had incorrect dimensions.
Was able to reproduce this when device was rotated during app launch. After rotation global layout listener callback gets invoked. Inside the callback current and previous orientations are compared. When a change is detected, orientation and dimension change events are sent to JS. It is assumed, when orientation changes, new dimensions are available immediately. This is not the case for window dimensions as they are retrieved from resources object which gets updated asynchronously after orientation change. In cases when app is doing a lot of work on the main thread, like app startup, it takes more time to update the resources object. And when orientation change is detected in global layout, resources object is not updated with new dimensions yet. This causes dimensions update to be sent to JS with old window dimensions.
Global layout listener callback does get invoked a second time when resources object is finally updated with new dimensions, but since orientation no longer changes, no event is sent to JS.

Fixed this by separating dimensions update from orientation update. Now RN keeps track of previous window and screen dimension values. When a change is detected, an event is sent to JS with updated dimensions. This ensures that whenever dimensions change, JS gets the updated values.

This has a side effect of sending dimension update twice in some cases.
One example is the case above where window dimensions take time to update, but screen dimensions are updated immediately. This will cause two events to be sent to JS. One for window dimensions and one for screen dimensions update.
Other change is that initial value for both window and screen fields is empty. Which results in first change to trigger an event. Previously initial orientation value was 0 which meant when app started in normal portrait orientation, first layout did not trigger a dimension update event. Now even first layout sends the event. This should not be an issue as it is to make sure dimensions in JS side are correct.

**Testing:**
Verified with a sample app that correct dimensions are available when app launches.
Verified that after orientation dimensions are updated.
Verified that in the scenario described above where window dimensions are updated later, we get correct dimension values in JS.
We have incorporated this fix into our app and have been testing it internally.

Ats Jenk
Microsoft Corp.

<!--
Thank you for sending the PR!

If you changed any code, please provide us with clear instructions on how you verified your changes work. In other words, a test plan is *required*. Bonus points for screenshots and videos!

Please read the Contribution Guidelines at https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md to learn more about contributing to React Native.

Happy contributing!
-->
Closes #15181

Differential Revision: D5552195

Pulled By: shergin

fbshipit-source-id: d1f190cb960090468886ff56cda58cac296745db
@variantf

This comment has been minimized.

variantf commented Oct 9, 2017

Waiting for this feature!!! Thanks a lot

@stale

This comment was marked as outdated.

stale bot commented Dec 8, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale label Dec 8, 2017

@Freddy03h

This comment has been minimized.

Contributor

Freddy03h commented Dec 8, 2017

No one target the iPad ? :(

@stale stale bot removed the Stale label Dec 8, 2017

@jamsch

This comment has been minimized.

Contributor

jamsch commented Dec 13, 2017

I'm having the same issue. Using a FlatList set to horizontal where every item has it's width set to the current dimensions. As a result, the items are now too wide because the window dimensions aren't correct.

One of my components listens for dimension changes and the view is updated to reflect those changes. while it works fine when not in split screen.

import { Dimensions } from 'react-native';
...
componentDidMount() {
  Dimensions.addEventListener('change', this._handleScreenDimensionsChange);
}
_handleScreenDimensionsChange = (dimensions: DimensionsChangeEvent) => {
  this.props.dispatch(dimensionsChange(dimensions.window));
};

image

@rogerkerse

This comment was marked as off-topic.

rogerkerse commented Dec 18, 2017

We have the same issue

@Axlle

This comment was marked as off-topic.

Axlle commented Jan 16, 2018

+1

@neiker

This comment has been minimized.

neiker commented Feb 8, 2018

Absolutely!

I solved this measuring the root component onLayout and sending the size to the store.

@stale

This comment was marked as outdated.

stale bot commented May 16, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale label May 16, 2018

@react-native-bot

This comment was marked as outdated.

Collaborator

react-native-bot commented May 16, 2018

It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest release, v0.55?

@stale stale bot removed the Stale label May 16, 2018

@mrtnzlml

This comment has been minimized.

mrtnzlml commented May 16, 2018

This is still a valid issue (latest RN release ^0.55.4). We have to use very nasty workarounds to make it really work (inspired by comments in this issue) and it's considered one of the biggest pains in our codebase.

@mrtnzlml mrtnzlml referenced this issue May 16, 2018

Closed

Biggest pains #466

@Freddy03h

This comment has been minimized.

Contributor

Freddy03h commented May 16, 2018

The issue is still valid !

@neiker

This comment was marked as off-topic.

neiker commented May 16, 2018

Still valid

@RoyalKingMomo

This comment has been minimized.

RoyalKingMomo commented Jul 26, 2018

PR Pending!!! Any updates on the admin side? When is thing being pulled into master?

@manoj2166

This comment has been minimized.

manoj2166 commented Sep 24, 2018

Hi,
you can use "onLayout" function on the root view of the component to calculate its dimensions. I have used and working fine here is sample code:

....//inner codes

onLayoutScreen = (e) => {
let width = e.nativeEvent.layout.width;
if(width != this.state.layoutWidth) {
this.setState({
layoutWidth: width
})
}

}
@rdonnelly

This comment has been minimized.

rdonnelly commented Sep 30, 2018

I've had a PR open for a few months now and am not sure how to proceed. So far, I'm not getting any traction on getting it merged. If anyone has an idea, please feel free to chime in.

For me, this issue is still valid.

@Gustash

This comment has been minimized.

Gustash commented Nov 10, 2018

This issue is still on the latest version of RN. We shouldn't have to apply workarounds for this. Split screen has been a feature of iOS since version 9. This seems like something that needs fixing urgently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment