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

Form type lazy load support #1443

Closed
sovetski opened this issue Apr 14, 2024 · 3 comments
Closed

Form type lazy load support #1443

sovetski opened this issue Apr 14, 2024 · 3 comments

Comments

@sovetski
Copy link
Contributor

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

Summary

In the templates\Form\fields.html.twig, we have this part:

        {%- if image_uri -%}
            <a href="{{ asset_helper is same as(true) ? asset(image_uri) : image_uri }}" download>
                <img src="{{ asset_helper is same as(true) ? asset(image_uri) : image_uri }}" alt="" />
            </a>
        {%- endif -%}

As the src attribute is hardcoded and we have not the ability to add a custom class name here, we can not use lazy image loading here.

I know that we generally use lazy load feature on a public pages where we have a lot of users, but sometimes, in admin pages where we can upload a lot of images, like 50+ images per product for ex, and we display them each time even if we don't see them, it means a lot of resources to consume for the server (imagine the website is a marketplace and people can edit their products X times, and sometimes they don't really need to see all images without scrolling).

My proposal is to add a new option to VichFileType like we have here: https://github.com/dustin10/VichUploaderBundle/blob/master/docs/form/vich_file_type.md

Maybe something like:

// ...
use Vich\UploaderBundle\Form\Type\VichFileType;

class Form extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        // ...

        $builder->add('genericFile', VichFileType::class, [
            'required' => false,
            'allow_delete' => true,
            'delete_label' => '...',
            'download_uri' => '...',
            'download_label' => '...',
            'asset_helper' => true,
            'src_alternative' => 'data-src', // <---- HERE
            'class_name' => 'lazy', // <---- HERE
        ]);
    }
}

In my case I am using https://github.com/verlok/vanilla-lazyload but there are more packages to do it probably, so with the options above we give the ability to adapt it to each library.

If it is ok for the maintainers, I can do a PR. Or if it is not ok, this is understandable.

A little message to other devs: If this issue is ever closed and you want the same functionality, don't hesitate to create a new issue or add comment here to let maintainers know.

@garak
Copy link
Collaborator

garak commented Apr 14, 2024

You still have the possibility of using your custom template.
It doesn't seem of so general interest to justify a feature here.

@sovetski
Copy link
Contributor Author

You still have the possibility of using your custom template.

The problem with templates, that they contains other logic that we don't need to change.
For example here fields.html.twig contains 52 lines, and if I copy this file to only edit 1 line, in the futur updates of the bundle, I have to check if the template changed and update it.

I completely understand that this is not the kind of problem that every person will encounter, I still created the issue in case it concerns other people, although I suspect not

@garak
Copy link
Collaborator

garak commented Apr 14, 2024

You don't need to redefine the entire template, overriding only a single block is possible (and advised)

@garak garak closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants