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

useResponsiveProps does not respect behavior on SSR #284

Open
kettanaito opened this issue Jan 8, 2020 · 2 comments · May be fixed by #288
Open

useResponsiveProps does not respect behavior on SSR #284

kettanaito opened this issue Jan 8, 2020 · 2 comments · May be fixed by #288
Assignees
Labels
bug scope:calculation This issue relates to the calculation logic

Comments

@kettanaito
Copy link
Owner

kettanaito commented Jan 8, 2020

What:

Props returned from useResponsiveProps does not respect common responsive props behaviors.

Why:

The responsive props matcher used on the server accounts only for the default breakpoint. So LgDown prop won't apply (although it should), because "lg" is not the default breakpoint.

const serverMatcher: MatcherFunction = (parsedProp) => {
  const { breakpoint } = parsedProp
  return breakpoint.isDefault && typeof window === 'undefined'
}

Environment:

  • atomic-layout version: 0.12.2

Reproduction steps:

import React from 'react'
import { useResponsiveProps } from 'atomic-layout'

// Declaration
const Component = (ownProps) => {
  const props = useResponsiveProps(ownProps)

  return <div {...props}>Text</div>
}

// Usage
<Component
  bgColorLdDown="red"
/>

During SSR the background color won't be present at all. It should be red.

Related issues:

@kettanaito
Copy link
Owner Author

kettanaito commented Jan 12, 2020

The problem

What causes this issue is the lack of matchMedia on the server. Since a media query string cannot be attempted to match (event against hard-coded window resolution—default breakpoint), there is no point of composing the media query string. Simple match of breakpoint.isDefault is not enough (see the issue's description).

Solution 1: Custom matchMedia

I think it would be great to have a single logic that asserts whether a breakpoint matches the screen. It can be an actual window (during runtime), or a hard-coded window-like object (dimensions only) for server-side. Since such solution is in-house, it could be fine grained towards specific requirements and needs. For example, such media matcher would accept a breakpoint Object, instead of a string, because that's how breakpoint data is stored in Atomic Layout. Compiling it back to string to parse into Object again would be an overkill.

The culprit here is the assertion of non-dimensional properties: aspect ratio (can be derived from dimensions), screen resolution, orientation, etc. Those are quite hard to reliably assert on a non-client environment with an imitated window object.

Certain breakpoint-related data can be assumed:

  • Aspect Ratio: derive from height and width
  • Orientation: Assume portrait
  • Resolution: Assume the default resolution

Solution 2: Direct area record analyzis

This is rather unusual, but pretty much any declarations of a single responsive prop can be represented in a "record", currently used for areas representation and responsive rendering analyzis:

export interface Template {
breakpoint: Breakpoint
behavior: BreakpointBehavior
areas: string[]
}

It's possible to compile a prop info into this kind of format, but I doubt it's necessary.

@kettanaito kettanaito linked a pull request Jan 12, 2020 that will close this issue
5 tasks
@kettanaito
Copy link
Owner Author

@ruhaise, in the end I followed a similar approach you've introduced when tackling #281. Only instead of match-media-mock I've decided to write a custom matcher function that would integrate with data structures Atomic Layout uses. The work is in progress.

@kettanaito kettanaito self-assigned this Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug scope:calculation This issue relates to the calculation logic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant