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

Defer icons to icons stack to reduce DOM count #191

Merged
merged 3 commits into from
May 11, 2022

Conversation

indykoning
Copy link
Contributor

@indykoning indykoning commented Apr 12, 2022

This pr allows you to defer the content of SVG elements to drastically reduce DOM element counts for complex icons which are used often on a page.
It's an easier to use alternative to using spritesheets to tackle #185

The way it works is simply by adding an attribute of defer to the icon e.g.

<x-icon-camera defer />

And loading the stack at the bottom of your layout

   ...
    <svg hidden class="hidden">
        @stack('bladeicons')
    </svg>
</body>
</html>

As an example here we're using the larger SVGs of the heroicon pack multiple times
image
The transparent cubes are 1K and the dollar sign is 880B
Loading these in every time we use the icon on a page would result in nearly 6K added to the page which is a lot for a couple of icons.
A category page with products using that icon for it's price might add 24K to the document for 24 paginated products.
Defering these icons will leave that at 1K and only take a couple of bytes for using the icons

In the HTML this will now result in
image

This will not break any exising usages of the icon pack since it requires you to add the defer attribute manually or add it under attributes in config/blade-icons.php

@indykoning indykoning marked this pull request as ready for review April 12, 2022 09:57
@royduin
Copy link
Contributor

royduin commented Apr 19, 2022

@driesvints, do you have some time to review this? 😇

@driesvints
Copy link
Member

Not yet I'm afraid :/

@royduin
Copy link
Contributor

royduin commented Apr 19, 2022

Np, take your time. Using the fork for now. Thanks 💯

Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Heya sorry for the long wait. I like this @indykoning. Just got one question here for now. I'll hopefully can try this out soon.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Dries Vints <dries@vints.io>
@driesvints
Copy link
Member

I didn't find time to actually try this out but since you two seem to have tested this already I'm going to merge and launch this. Thanks both 👍

@driesvints driesvints merged commit 57a7c41 into blade-ui-kit:1.x May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants