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

useDeviceOrientation returns incorrect initial values #210

Open
tomas223 opened this issue Oct 19, 2020 · 3 comments
Open

useDeviceOrientation returns incorrect initial values #210

tomas223 opened this issue Oct 19, 2020 · 3 comments

Comments

@tomas223
Copy link

Initial values returned by this hook can be incorrect based od how was app started and when hook mounted.

Example:
App is started in portrait, then orientation is chaged to landscape and after that this hook is mounted. It will show that device is in landscape.
Not sure if this is by design or it's a bug.

My current solution looks like this:

function getDimensions() {
  return Dimensions.get("screen");
}

...
function useMyDeviceOrientation() {
...
  const [orientation, setOrientation] = useState({
    portrait: isOrientationPortrait(getDimensions()),
    landscape: isOrientationLandscape(getDimensions()),
  });
...
}
@tomas223
Copy link
Author

tomas223 commented Oct 19, 2020

I see there is another opened issue for this. 8cfe3a5

I have updated solution taking into considetion points from solution above that is stucked.

import { useEffect, useState, useCallback } from "react";
import { Dimensions, ScaledSize } from "react-native";

function getDimensions() {
  return Dimensions.get("screen");
}

export type Orientation = "landscape" | "portrait";

const getOrientation = ({
  width,
  height,
}: {
  width: number;
  height: number;
}): Orientation => (width <= height ? "portrait" : "landscape");

function useMyDeviceOrientation(): Orientation {
  const [orientation, setOrientation] = useState(getOrientation(getDimensions()));

  const onChange = useCallback(({ screen: scr }: { screen: ScaledSize }) => {
    setOrientation(getOrientation(scr));
  }, []);

  useEffect(() => {
    Dimensions.addEventListener("change", onChange);

    return () => {
      Dimensions.removeEventListener("change", onChange);
    };
  }, [onChange]);

  return orientation;
}

export default useMyDeviceOrientation;

Then it can be used like this:

const landscape = useMyDeviceOrientation() === "landscape";

Let me know if I can start testing and prepare pull request.

@LinusU
Copy link
Member

LinusU commented Nov 4, 2020

A good first step that's non-controversial would be to move the const screen = Dimensions.get('screen') inside the component so that the value is always fresh. A PR for that could be accepted right away.

If we want to change the return value we need to have some more discussions.

If we change to a string, what should we return when the app is running on a square canvas?

@tomashornak
Copy link

tomashornak commented Nov 5, 2020

  1. In that case I would write useState hook this way:
  const [orientation, setOrientation] = useState(() => {
    const screen = Dimensions.get("screen");
    return {
      portrait: isOrientationPortrait(screen),
      landscape: isOrientationLandscape(screen),
    };
  });

I know it's little bit ugly but this is only place where Dimensions.get("screen") needs to be called.

I would also move functions isOrientationPortrait & isOrientationLandscape out of hook.

  1. Regarding changing return value. You are right. String portrait vs landscape is probably not best idea.
    My intention was to avoid unnecessary re-renders triggered by calling setState with string instead of object when screen dimensions change but not orientation. But I maybe overthought it as this scenario doesn't usually happen.

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

No branches or pull requests

3 participants