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.

Sign up for GitHub

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

264-fix: load images for smal window #333

Closed
wants to merge 16 commits into from
Closed

264-fix: load images for smal window #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fc82b28
fix: 264 - add svg check
ya6 Jun 22, 2024
2105548
fix: 264 - change condition
ya6 Jun 22, 2024
3cc7166
fix: 264 - prevent srcset
ya6 Jun 24, 2024
09e9152
feat: 264 - add check size utilite
ya6 Jun 26, 2024
95c4295
refactor: 264 - del logs
ya6 Jun 26, 2024
ecb63a6
fix: 264 - condition parameter
ya6 Jun 26, 2024
77d5708
fix: 264 - logical operator
ya6 Jun 26, 2024
8320697
refactor: 264 - add constant, make condition for not svg extention
ya6 Jul 2, 2024
278a550
refactor: 262 - add synchronous approach for check image size
ya6 Jul 2, 2024
b81c5d0
refactor: 262 - add loading img check
ya6 Jul 2, 2024
650f57f
refactor: 264 - remove svg files when creating srcSet
ya6 Jul 3, 2024
ab14a3c
fix: 264 - remove log
ya6 Jul 3, 2024
903417e
refactor: 264 - rename & add return type to function
ya6 Jul 3, 2024
00a2312
refactor: 264 - remove default export
ya6 Jul 4, 2024
d6453f7
fix: 264 - return different file formats for local development
ya6 Jul 4, 2024
c57e298
Merge branch 'main' into fix/264-unknown-error-when-the-browser-windo…
ansivgit Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified src/shared/assets/mentors-wanted.webp
View file
Open in desktop
Binary file not shown.
13 changes: 8 additions & 5 deletions src/shared/ui/image/image.tsx
View file
Open in desktop
Copy link
Member

@Quiddlee Quiddlee Jun 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not fix the errors.

From the image optimization doc
image

There is a clear statement that if an image is already small, the 2 smaller sizes not gonna be created.

Therefore when the browser loading the image that is missing it throws 404 error and Image fallbacks to basic src instead of srcSet

image

Because of this behavior, errors occurs.

We have 3 variants on how to fix this:

  1. We can generate two more images with different names but the same size as the original. For example, if the existing logo is named "RS logo" and is 90x90 pixels, we would generate two more copies named "image-name-768" and "image-name-425." These additional images would essentially be placeholders.

  2. This option is more complex. It involves checking all the created images, parsing the build code, and then modifying the srcSet attribute for each image. The modification would be to remove any references to responsive image sizes that are missing.

  3. We can ignore the errors, or somehow clear the console after the image performed fallback

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not fix the errors.

From the image optimization doc image

There is a clear statement that if an image is already small, the 2 smaller sizes not gonna be created.

Therefore when the browser loading the image that is missing it throws 404 error and Image fallbacks to basic src instead of srcSet

image

Because of this behavior, errors occurs.

We have 3 variants on how to fix this:

  1. We can generate two more images with different names but the same size as the original. For example, if the existing logo is named "RS logo" and is 90x90 pixels, we would generate two more copies named "image-name-768" and "image-name-425." These additional images would essentially be placeholders.
  2. This option is more complex. It involves checking all the created images, parsing the build code, and then modifying the srcSet attribute for each image. The modification would be to remove any references to responsive image sizes that are missing.
  3. We can ignore the errors, or somehow clear the console after the image performed fallback

I agree with the proposed possible solutions.
If generate a srcSet for them, about 100 more files are added and the size of the application grows
The second option to increase the complexity of an already complex solution will not be optimal
There is another solution to replace the files with svg, but I still don’t think this is the optimal solution.

Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@
import { FC, useState } from 'react';
import { IS_DEV } from './constants';
import { DecodingAttr, FetchPriorityAttr, ImageProps, LoadingAttr } from './types';
import convertToWebp from './utils/convertToWebp';
import generateSizes from './utils/generateSizes';
import generateSrcSet from './utils/generateSrcSet';
import { checkIfSvg } from './utils/checkIfSvg';
import { convertToWebp } from './utils/convertToWebp';
import { generateSizes } from './utils/generateSizes';
import { generateSrcSet } from './utils/generateSrcSet';

const Image: FC<ImageProps> = ({ alt, src = '', lazy = 'true', ...props }) => {
const srcWebp = convertToWebp(src);
const [srcSet, setSrcSet] = useState(() => (IS_DEV ? undefined : generateSrcSet(srcWebp)));
const [sizes, setSizes] = useState(() => (IS_DEV ? undefined : generateSizes()));
const isSvg = checkIfSvg(srcWebp);

ansivgit marked this conversation as resolved.
Show resolved Hide resolved
const [srcSet, setSrcSet] = useState(() => (IS_DEV || isSvg ? undefined : generateSrcSet(src)));
const [sizes, setSizes] = useState(() => (IS_DEV || isSvg ? undefined : generateSizes()));

const isLazy = lazy === 'true';
const loading: LoadingAttr = IS_DEV ? 'eager' : isLazy ? 'lazy' : 'eager';
Expand Down Expand Up @@ -45,4 +48,4 @@
);
};

export default Image;

Check warning on line 51 in src/shared/ui/image/image.tsx

View workflow job for this annotation

GitHub Actions / CI 

Exporting 'default' is restricted
6 changes: 6 additions & 0 deletions src/shared/ui/image/utils/checkIfSvg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const checkIfSvg = (imgUrl: string): boolean => {
if (imgUrl.endsWith('.svg')) {
return true;
}
return false;
};
4 changes: 1 addition & 3 deletions src/shared/ui/image/utils/convertToWebp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const convertToWebp = (src: string) => {
export const convertToWebp = (src: string) => {
const srcLowCase = src.toLowerCase();

if (
Expand All @@ -10,5 +10,3 @@ const convertToWebp = (src: string) => {
return src;
return `${src.slice(0, src.lastIndexOf('.'))}.webp`;
};

export default convertToWebp;
4 changes: 1 addition & 3 deletions src/shared/ui/image/utils/generateSizes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { DESKTOP_W, MOBILE_W, TABLET_W } from '../constants';
* Generates responsive sizes for Img element, based on the env breakpoints
* @returns {string} - Returns the sizes string
*/
const generateSizes = () => {
export const generateSizes = () => {
return `(max-width: ${MOBILE_W}px) ${MOBILE_W}px, (max-width: ${TABLET_W}px) ${TABLET_W}px, ${DESKTOP_W}px`;
};

export default generateSizes;
4 changes: 1 addition & 3 deletions src/shared/ui/image/utils/generateSrcSet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import { DESKTOP_W, MOBILE_W, TABLET_W } from '../constants';
* @param {string} src - An src value to generate the srcset from
* @returns {string} - Returns the processed srcset
*/
const generateSrcSet = (src: string) => {
export const generateSrcSet = (src: string) => {
const srcNoExtension = src.slice(0, src.lastIndexOf('.'));
const srcSet = `${srcNoExtension}-${MOBILE_W}.webp ${MOBILE_W}w, ${srcNoExtension}-${TABLET_W}.webp ${TABLET_W}w, ${src} ${DESKTOP_W}w`;

return srcSet;
};

export default generateSrcSet;
Loading