Skip to content
New issue

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

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

Already on GitHub? # to your account

Bug and potential changes? #2

Open
karland opened this issue Feb 17, 2020 · 0 comments
Open

Bug and potential changes? #2

karland opened this issue Feb 17, 2020 · 0 comments

Comments

@karland
Copy link

karland commented Feb 17, 2020

Hi, your library is rather nice. Thank you for the effort.

For my own purposes, I have made some (API and param) changes and wonder if you would be interested? I know, these are big changes. So, feel free to close.

  1. This seems to be a bug: it should read width instead of height as your are in landscape mode:
    https://github.com/applike/responsive-react/blob/eb601c0aa78492f359d387bd0291d43a252a94f5/src/utilResponsive.js#L64-L65

  2. In the README.md under ### Approach 2 and ### Additional helper function I suggest to
    import {...} from "react-responsive".

  3. I added constants

export const PORTRAIT = "PORTRAIT"
export const LANDSCAPE = "LANDSCAPE"
export const MOBILE = "MOBILE"
export const MOBILE_SMALL = "MOBILE_SMALL"
export const MOBILE_MEDIUM = "MOBILE_MEDIUM"
export const MOBILE_LARGE = "MOBILE_LARGE"
export const TABLET = "TABLET"
export const IPAD_PRO = "IPAD_PRO"
export const LAPTOP = "LAPTOP"
export const LAPTOP_SMALL = "LAPTOP_SMALL"
export const LAPTOP_MEDIUM = "LAPTOP_MEDIUM"
export const LARGE_SCREEN = "LARGE_SCREEN"
export const SUPER_LARGE_SCREEN = "SUPER_LARGE_SCREEN"
  1. Update screen sizes of smartphones and take acount of laptops with 1920:
export const DeviceWidthObject = {
  // MobileSmall: { max: 320, min: 0 },
  MOBILE_SMALL: { max: 480, min: 0 }, // e.g. iPhone4
  // MobileMedium: { max: 375, min: 321 },
  MOBILE_MEDIUM: { max: 640, min: 481 }, // e.g. iPhone5
  MOBILE_LARGE: { max: 767, min: 641 }, // e.g. Pixel 2, iPhone X
  TABLET: { max: 991, min: 768 },
  LAPTOP_SMALL: { max: 1024, min: 992 },
  // LaptopLarge: { max: 1440, min: 1025 },
  LAPTOP_MEDIUM: { max: 1440, min: 1025 },
  // LargerThanLaptop: { max: 2560, min: 1441 },
  LARGE_SCREEN: { max: 2560, min: 1441 },
  SUPER_LARGE_SCREEN: { max: 999999, min: 2561 }
}
  1. You can save 1/3 of the code in utilResponsive.js if you always virtually swap the device into landscape mode to determine its size.

  2. I have made boolean values. e.g. isMobileDevice, boolean and not a function

export const isMobileDevice = (() => {
  const deviceInformation = getDeviceTypeInfo()
  return deviceInformation.deviceType === "Mobile"
})()
  1. OPTIONAL: Added some additional precision booleans:
import {
  LANDSCAPE,
  LAPTOP_SMALL,
  LAPTOP_MEDIUM,
  LARGE_SCREEN,
  SUPER_LARGE_SCREEN,
  MOBILE_SMALL,
  MOBILE_MEDIUM,
  MOBILE_LARGE,
  TABLET,
  IPAD_PRO,
  getDeviceTypeInfo
} from "./utilResponsive"

const sizeDevice = () => {
  const { deviceTypeVariant: deviceType, orientation } = getDeviceTypeInfo()
  const isLandscape = orientation === LANDSCAPE
  const isPortrait = !isLandscape
  const isSmartphoneSmall = deviceType === MOBILE_SMALL
  const isSmartphoneMedium = deviceType === MOBILE_MEDIUM
  const isSmartphoneLarge = deviceType === MOBILE_LARGE

  const isMaxSmartphone =
    isSmartphoneSmall || isSmartphoneMedium || isSmartphoneLarge

  const isSuperLargeScreen = deviceType === SUPER_LARGE_SCREEN

  const isMinLargeScreen = deviceType === LARGE_SCREEN || isSuperLargeScreen

  const isMinLaptop =
    deviceType === LAPTOP_SMALL ||
    deviceType === LAPTOP_MEDIUM ||
    isMinLargeScreen

  const isMinTablet =
    deviceType === TABLET || deviceType === IPAD_PRO || isMinLaptop

  const isMinSmartphoneLarge = isSmartphoneLarge || isMinLaptop
  const isMinSmartphoneMedium = isSmartphoneMedium || isSmartphoneLarge

  return {
    isPortrait,
    isLandscape,
    isSmartphoneSmall,
    isSmartphoneMedium,
    isSmartphoneLarge,
    isMaxSmartphone,
    isMinSmartphoneMedium,
    isMinSmartphoneLarge,
    isMinTablet,
    isMinLaptop,
    isMinLargeScreen,
    isSuperLargeScreen
  }
}

What do you think?

@karland karland changed the title Bug and potenial changes? Bug and potential changes? Feb 17, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant