Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

First render uses the first breakpoint it finds #8

Closed
SoaresMG opened this issue Jul 29, 2020 · 13 comments
Closed

First render uses the first breakpoint it finds #8

SoaresMG opened this issue Jul 29, 2020 · 13 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@SoaresMG
Copy link
Contributor

Issue Report

Right now the first time ResponsiveProvider runs it will use the first option on breakpoints (which normally is the smallest one).

Expected Behavior

It should render the correct breakpoint of first usage.

Actual Behavior

There is a jump from the initial breakpoint to the correct one after useEffect runs the first time.

Steps to Reproduce the Issue

  1. Check example
  2. While being on desktop res it will quickly change from mobile to desktop (there is a visual jump)
@SoaresMG SoaresMG added bug Something isn't working good first issue Good for newcomers labels Jul 29, 2020
@joaopedrodcf
Copy link

joaopedrodcf commented Jul 29, 2020

This Problem is originated because we don't know the viewport of the client and so we fallback to mobile, and then on the client side it calculates the correct viewport and changes as can be seen on your example.

@joaopedrodcf
Copy link

Been investigating other libraries and seems like they use the ua-parser-js to validate the initial breakpoint even if the user agent is not trustworthy all the times.

We could use it as well to calculate our initial breakpoint what do you guys think ?

Because today is always mobile so when we don't want to take the risk to have the mobile showing in desktop we use the isCalculated to prevent it from been shown in SSR.

@joaopedrodcf
Copy link

As an example: material UI media query ssr

Here they suggest to their clients to use the user-agent-js in their side.

@SoaresMG
Copy link
Contributor Author

SoaresMG commented Aug 1, 2020

That could be a good idea, by using the agent on both server and client side, it has a lot more chances of matching the real device.

It would also be SSR friendly.

However we would still need to run the logic in useEffect to be 100% sure it will be correctly displayed.

What do you think? @dinospereira @sofiacteixeira @themariamarques

@SoaresMG SoaresMG added the help wanted Extra attention is needed label Aug 1, 2020
@joaopedrodcf
Copy link

Yeah the idea was to have the ua-parser-js calculating the initial currentBreakpoint and then update it in the useEffect when the client side knows the width of the client.

What also would be cool is to see the number of times this calculates the correct breakpoint.

@SoaresMG
Copy link
Contributor Author

SoaresMG commented Aug 5, 2020

@joaopedrodcf are you available to create a PR with that?

@joaopedrodcf
Copy link

Yeah 🔥
I was just waiting responses from the rest of the mantainers, but if all cool sign it to me.

@SoaresMG
Copy link
Contributor Author

SoaresMG commented Aug 5, 2020

@dinospereira @sofiacteixeira @themariamarques what do you think?

@sofiacteixeira
Copy link

It looks like a good approach! Thanks @joaopedrodcf and @SoaresMG!

@dinospereira
Copy link

I think its a good approach, thanks @joaopedrodcf

@joaopedrodcf
Copy link

joaopedrodcf commented Aug 20, 2020

Okay guys just to give an update on this so I been experimenting with ua-parser-js one thing I noticed is that it can only tell you if it considers mobile or desktop

So something like this could be made

const ssrInferedMediaType = device.type === "mobile" ? "xs" : "xl";

And we could take advantage of the MR #28 in here to get the mobileBreakpoint as @SoaresMG suggested to me through a DM.

Another option instead of integrating the library ourselves let our clients use it and update the readme with how to do it https://codesandbox.io/s/admiring-dubinsky-6dm0u?file=/src/index.js:494-562 this sandbox show how to do it.
Basically we can use the initialMediaType to take the value depending on the device that we want.

const ssrInferedMediaType = device.type === "mobile" ? "xs" : "xl";

const mockProps = {
  initialMediaType: ssrInferedMediaType,
  breakpoints,
  breakpointsMax
};

const rootElement = document.getElementById("root");
ReactDOM.render(
  <React.Fragment>
    <React.StrictMode>
      <ResponsiveProvider {...mockProps}>
        <App />
      </ResponsiveProvider>
    </React.StrictMode>
  </React.Fragment>,
  rootElement
);

Opinions?

@joaopedrodcf
Copy link

what are your thoughts on this guys @SoaresMG @dinospereira @sofiacteixeira @themariamarques ?

@gsferreira
Copy link

Closed due to project archival #76

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants