react-native-community/hooks

useDeviceOrientation returns incorrect initial values

tomas223 opened this issue · 3 comments

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()),
  });
...
}

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.

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?

  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.