264-fix: load images for smal window #333
Conversation
There was a problem hiding this comment.
This will not fix the errors.
From the image optimization doc
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
Because of this behavior, errors occurs.
We have 3 variants on how to fix this:
-
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.
-
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.
-
We can ignore the errors, or somehow clear the console after the image performed fallback
There was a problem hiding this comment.
This will not fix the errors.
From the image optimization doc
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
srcinstead ofsrcSet
Because of this behavior, errors occurs.
We have 3 variants on how to fix this:
- 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.
- 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.
- 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.
WalkthroughThe recent updates primarily involve enhancements to the Changes
Sequence DiagramsSequence diagrams are not applicable for these changes as they mainly include small adjustments and refactoring rather than new features or complex control flows. Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
In my opinion the approach works like a "silver bullet" with the presence of this error
|
|
Lighthouse Report:
|
Qodana for JSIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 82 dependenciesThird-party software listThis page lists the third-party software dependencies used in project
Contact Qodana teamContact us at qodana-support@jetbrains.com
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
ansivgit
requested review from
andron13,
dzmitry-varabei and
SpaNb4
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/shared/ui/image/image.tsx (1 hunks)
- src/shared/ui/image/utils/checkIfSvg.ts (1 hunks)
- src/shared/ui/image/utils/convertToWebp.ts (2 hunks)
- src/shared/ui/image/utils/generateSizes.ts (1 hunks)
- src/shared/ui/image/utils/generateSrcSet.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- src/shared/ui/image/utils/checkIfSvg.ts
- src/shared/ui/image/utils/convertToWebp.ts
Additional comments not posted (5)
src/shared/ui/image/utils/generateSizes.ts (1)
7-7: LGTM!The change to a named export for
generateSizesis appropriate.src/shared/ui/image/utils/generateSrcSet.ts (1)
8-8: LGTM!The change to a named export for
generateSrcSetis appropriate.src/shared/ui/image/image.tsx (3)
5-8: LGTM!The updated import statements appropriately include the
checkIfSvgfunction and update other utilities to named exports.
12-13: LGTM!The addition of the
checkIfSvgfunction ensures that SVG files are correctly identified.
14-15: Conditional Initialization of StatesThe conditional initialization of
srcSetandsizesbased on theIS_DEVflag andcheckIfSvgensures that SVG files are handled appropriately. This change is logical and aligns with the bug fix objectives.
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
In my opinion the approach works like a "silver bullet" with the presence of this error
After trying several approaches, in the latest version of the files I suggest:
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
Summary by CodeRabbit
New Features
srcSetandsizesattributes for better image loading capabilities.Improvements