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

Views & Default Styles Update - NO JS #59

Merged
merged 20 commits into from
Feb 28, 2024
Merged

Conversation

mrdoinel
Copy link
Contributor

@mrdoinel mrdoinel commented Jan 29, 2024

Major update regarding how the markup is being generated :

  1. Remove the JS script and the logic to lazy load images with JS in here.
  2. Picture element : Remove noscript tag
  3. New Twill-Image config : Add tailwind_css config to tell if the default style should be added using tailwind CSS classes or regular inline styles. This will avoid the output of repetitive inline styles attributes markup when using tailwind CSS in your application.
  4. New render option : Add imageSizer boolean option to tell if it should render the sizer markup : a div under the image that push the zone to a certain percentage. If imageSizer is false, you will need to properly set wrapper aspect-ratio using wrapperClass option which is making sizing easier for responsive aspect-ratio for example.
  5. New render option : Add imageClass string option to set css classes on the img tag.
  6. Remove the layout option

This change how the wrapper element around image is added and how the default style is added : the wrapper and the default style around the image tag is only added if the Placeholder is setup or if ImageSizer option is set to true.

Because I am removing the JS script and logic, this will require a breaking change version: 2.0.0

@mrdoinel mrdoinel requested a review from ptrckvzn January 29, 2024 09:55
@mrdoinel mrdoinel changed the title Views & Default Styles Update Views & Default Styles Update - NO JS Jan 29, 2024
@mrdoinel mrdoinel requested a review from ifox January 29, 2024 13:23
@mrdoinel mrdoinel changed the base branch from 1.x to 2.x January 29, 2024 13:24
@mrdoinel
Copy link
Contributor Author

All changes have been reviewed in the context of an actual project.

@mrdoinel mrdoinel marked this pull request as ready for review January 31, 2024 10:07
@ptrckvzn
Copy link
Member

Hi @mrdoinel, this looks great and those were needed updates.

I have two comments or questions. The first one is about the tailwind support. The reason to use inline styling was to avoid depending on an external stylesheet, so that the image layout can be applied faster. You didn't notice any jumping content while applying styles when using Tailwind?

The other is about the layout option. What was the reason to remove it? I think it must be that it's rarely being used?

Thanks!

@ptrckvzn
Copy link
Member

@mrdoinel if you have a minute to test, here's the changes I did to add config options:

  • image_sizer option, values can be 'auto', true or false (auto is the default and will render sizer if lqip is true)
  • inline_styles option, by default is true to allow the package to be used out-of-the-box, setting to false will not use inline styles, but custom classes
  • custom_classes option, those defines classes (tailwind css) for main, wrapper and placeholder

@ptrckvzn
Copy link
Member

Changes to the way inline styles and css classes work:

  • mode can be set to inline-styles, classes or both
  • inline_styles and classes will be applied based on the mode option

Background color option has been removed as it can simply be added to either inline styles or classes.

@ptrckvzn ptrckvzn merged commit 2b106a2 into 2.x Feb 28, 2024
@ptrckvzn ptrckvzn deleted the update-views-logic-no-js branch February 28, 2024 19:04
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

Successfully merging this pull request may close these issues.

2 participants