search

tjikko-studio / components

Component library for Tjikko Studio apps
0 stars 0 forks source link

Avoid layout shifts #63

Open shawninder opened 3 years ago

shawninder commented 3 years ago

When a Primary Block loads, the slower-loading image introduces layout shift.

In general this can be easily avoided by using next/Image, which among other things forces you to set a specific width and height for all your images.

Check for this problem elsewhere too, like in the BlockIcon block perhaps?

shawninder commented 3 years ago

Does next/Image require us to whitelist domains from which images can be loaded? I forget, but in any case the immediate need is to avoid the layout shift, we can move to next/Image if we want later if it's too complicated for now… Still, wether we use it or not, avoiding layout shift does seem to imply hard-coding the image size, but in fact we can get the size on the Kirby side when the image is uploaded or its URL specified, and then pass that information down for the components to be able to add a specific width and height on their img components.

starjdave commented 3 years ago

I'm trying to get image size on kirby page, but it seems to be issues with configuration. I see 2 identical files: backend/site/blueprints/files/default.yml backend/site/plugins/tjikko-blocks/blueprints/files/Default.yml

And Primary/Secondary blocks are using first one, but it looks like we were expecting it should use second one. I've tried different configuration options, but couldn't make Kirby to use second one. If you can assist here, it should help.

I've added width/height to first one, couldn't get it from API. How doest Kirby handle it?

MediaProps in components/src/parts/Media.tsx are completely different from that we are getting from API.

shawninder commented 3 years ago

I'm trying to get image size on kirby page, but it seems to be issues with configuration. I see 2 identical files: backend/site/blueprints/files/default.yml backend/site/plugins/tjikko-blocks/blueprints/files/Default.yml

And Primary/Secondary blocks are using first one, but it looks like we were expecting it should use second one. I've tried different configuration options, but couldn't make Kirby to use second one. If you can assist here, it should help.

In general we want things to go in the plugin to make it easily reusable for our next clients, but sometimes that isn't exactly possible due to how Kirby works… I don't know how Kirby handles having the same file in both locations, but I think even for our own sake we should try having a single one of those files. I'll have a look and see if I can determine the best way forward.

I've added width/height to first one, couldn't get it from API. How doest Kirby handle it?

I don't know how Kirby expects us to handle this kind of thing… Maybe we can get access to the width and height of the image as it is uploaded (or as a URL is saved)? Then we could save it to a field whose value could be fetched via the API? Not sure.

MediaProps in components/src/parts/Media.tsx are completely different from that we are getting from API.

We can change how MediaProps works, but we can also translate from API data to component props in the backend. We wouldn't want to be forced by Kirby into a weird set of props for our components. Eventually, we'll want to use these same components outside of Kirby…

starjdave commented 3 years ago

Ok, looks like I've found much much easier way to fix layout shifting :) Just via fixing styles. I've tested different layouts and cases, seems to work fine, no need to specify width/height manually and/or use next/images.

But we can switch to next/images later, cause it's best way to handle images via next.js, to do what we need to find a way how easier defined with/height for images we want to use, including responsive staff

shawninder commented 3 years ago

As expected this doesn't work 100%. The primary block, for example, want to center the text vertically with respect with the image. So although the PR was a quick and easy partial fix and was merged, this issue still remains.

starjdave commented 3 years ago

You checked that locally or in production?

shawninder commented 3 years ago

Locally via Storybook. Production is broken, remember? ;)

shawninder commented 3 years ago

https://user-images.githubusercontent.com/484730/135091711-a7493ecb-8fdf-41d6-b331-2e14ec2995f0.mov

Here is a screen recording of the layout shift. I guess on some setups you may have to throttle your network speed to comfortably observe this, or alternatively you could also go into Storybook and force a super heavy image… On my network, with my computer, there is no need to artificially slow things down: I get the layout shift everytime