bluesky-social / social-app

The Bluesky Social application for Web, iOS, and Android
https://bsky.app
MIT License
8.32k stars 1.11k forks source link

Viewport breaks at ~1200px #1267

Closed raykyri closed 1 year ago

raykyri commented 1 year ago

Describe the bug

The compose box is broken at 1200px width.

There may be other bugs related to screen width too - timeline rendering was broken for a day or so until I resized to 1300px width, but then was fixed after switching back. I can't tell if this was because of some client side query malfunctioning or because the feed generator just happened to fix itself at the same time.

To Reproduce

  1. Set window to ~1200px width. On a 14" Mac this can be done by just enabling vertical tabs.
  2. Go to a post

Expected behavior

Compose box appears in regular place.

Screenshots

image

Details

Additional context

mozzius commented 1 year ago

(Closing my issue in favour of this one)

The exact breakpoint is 1000px, and by my estimation there are ~93 other places the bug is present - the web app has conditional behaviour based on screen size, but does not respond to changes in screen size.

The problem is here:

// src/platform/detection.ts
import {Platform} from 'react-native'

export const isIOS = Platform.OS === 'ios'
export const isAndroid = Platform.OS === 'android'
export const isNative = isIOS || isAndroid
export const isWeb = !isNative
export const isMobileWeb =
  isWeb &&
  // @ts-ignore we know window exists -prf
  global.window.matchMedia('only screen and (max-width: 1000px)')?.matches
export const isDesktopWeb = isWeb && !isMobileWeb

isDesktopWeb and isMobileWeb should be replaced by a hook that looks something like this:

export const useWebScreenSize = () => {
  const [isMobile, setIsMobile] = useState(
    isWeb &&
      global.window.matchMedia('only screen and (max-width: 1000px)')?.matches,
  )

  useEffect(() => {
    if (!isWeb) return
    const mediaQuery = global.window.matchMedia(
      'only screen and (max-width: 1000px)',
    )
    const listener = () => setIsMobile(mediaQuery.matches)
    mediaQuery.addEventListener('change', listener)
    return () => mediaQuery.removeEventListener('change', listener)
  }, [])

  return useMemo(
    () => ({
      isMobile,
      isDesktop: isWeb && !isMobile,
    }),
    [isMobile],
  )
}
pfrazee commented 1 year ago

Yeah there's two things that need to happen:

  1. We need to introduce dynamic hooks for viewport size akin to what @mozzius is sharing
  2. We need to actually style desktop to handle the different viewport sizes, which is a whole bundle of work
mozzius commented 1 year ago

@pfrazee was this fixed in v1.49 then? Patch notes implies it was

pfrazee commented 1 year ago

Yep!