Skip to content
This repository was archived by the owner on Dec 23, 2020. It is now read-only.

Add/abstract filtering of tags #9

Closed
wants to merge 3 commits into from

Conversation

azaozz
Copy link
Contributor

@azaozz azaozz commented Jan 22, 2020

Makes the implementation a lot more flexible and easier to expand in the future.

  • Splits (and abstracts) wp_add_lazy_load_attributes(). Introduces _wp_filter_html_tags() that makes it possible to search for any tag, and fires a filter depending on the tag name.
  • Introduces wp_get_tags_to_filter() that has a filterable list of the default tags (currently only img).
  • Adds wp_add_lazy_loading_to_img_tags() used with the new filter wp_filter_img_tags fired when img tags are found in _wp_filter_html_tags().
  • Introduces use of context where possible. This makes it a lot easier to use for specific cases.

See #4.

- Add filter for each tag.
- Make it easy to filter other tags and/or add more attributes in the future.
@azaozz
Copy link
Contributor Author

azaozz commented Jan 23, 2020

Looking at this again, code is good but wondering if this functionality should be added in the first place :)

It allows "searching" the content for any (opening) HTML tags, then runs a tag-specific filter on the found tags. General drawback is that it will only be used to find img for now and eventually for iframes.

It may be useful in the future if there's an attribute we'd want to auto-add similarly to "loading", or perhaps to remove an attribute or a class name as a "last resort" fix in some cases. It may also be useful for some plugins as there will be only one preg_replace_callback(), not several. But all of these cases are quite "theoretical" for now, perhaps best to add it when it's needed more.

@felixarntz
Copy link
Member

@azaozz I think we can keep this simple for now and limited to img tags (see #3). As long as the public interface we introduce (filters) is flexible for the future, we should be good. In other words, I think we should keep the wp_lazy_loading_enabled contextual including a variable $tag_name, while for now this would only ever be img.

@azaozz
Copy link
Contributor Author

azaozz commented Jan 23, 2020

I think we can keep this simple for now...

Yeah, thinking the same. Can add the extended support when it's "really needed" :)

@azaozz azaozz closed this Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants