Add new image container component and deprecate legacy image variants

[jmuzina]

Done

Added several classes that will help unify the appearance of images in projects that use Vanilla, and deprecated some styles that are no longer part of branding.

• p-image-container-- classes for setting element aspect ratio.
  • p-image-container--16-9
  • p-image-container--3-2
  • p-image-container--2-3
  • p-image-container--cinematic
  • p-image-container--square
• p-image-container .is-highlighted for distinguishing an image from its surroundings by applying a slight underlay.
• Deprecated image modifiers p-image--bordered and p-image--shadowed

Closes: #4876 WD-11412

QA

• Review updated Images pattern documentation
• Review updated release notes for 4.12.0

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

• PR should have one of the following labels to automatically categorise it in release notes:
  • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
• Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
  • if CSS class names are not changed it can be bugfix relesase (x.x.X)
  • if CSS class names are changed/added/removed it should be minor version (x.X.0)
  • see the wiki for more details
• Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

Background highlight:

Aspect ratio:

[webteam-app]

Demo

Jenkins

demos.haus

[lyubomir-popov]

hi - I think we can drop u-image-container--padded as I made an issue to keep hr margins in all cases, so we no longer need to do this. And I think the name of 2-3-4 should be --2.4 (was using 2.34 but the correct one is 2.4 after reading this )

[jmuzina]

As @lyubomir-popov mentioned, u-image-container--padded may be removed as we could be deprecating p-rule. See #5096 to track p-rule deprecation progress.

[jmuzina]

I think the name of 2-3-4 should be --2.4 (was using 2.34 but the correct one is 2.4 after reading this )

Adjusted this to use the class name you described and switched to aspect-ratio: 1/2.4.

While it is syntactically possible for us to use u-aspect-ratio--2.4, the period in the class name has to be escaped for the CSS to be valid (u-aspect-ratio--2\.4). Then it can be used as normal.

This causes a linting failure due to our linter configuration.

@bartaz @pastelcyborg What do you think is the most semantically clear way to name this class? I don't think we should alter our linting rules for this. Perhaps u-aspect-ratio--theater as suggested by the link @lyubomir-popov sent?

[bartaz]

I still think we should not introduce them as utilities if they are meant for images only.

Utilities should be a last resort class name that clearly can be applied in ANY context. These from their description seem like are designed to be used with images and images only.

Our current image styling is quite dated (we probably don't want to use the p-image--bordered or p-image--shadowed anymore https://vanillaframework.io/docs/patterns/images), but maybe it's an opportunity to clean that up and provide a consistent image component with options that we actually need and use.

Creating those image-only class names as utilities is a bad precence in my opition.

@lyubomir-popov What kind of images styles do we need to support?

I see we have images that cover all available space, and illustrations that have some kind of highlight background around them. Any other "background" styles?

The other variable is aspect ratio that is quite well defined.

Is there anything else about images we want to control?

How I see it is that we could possibly have some kind of p-image component with any options needed.

So something like:

<figure class="p-image--aspect-ratio-3-4 is-highighted><img class="p-image__img" /></figure>

[pastelcyborg]

Ditto @bartaz comment about them not being utility classes. As a general rule, I believe we should be rigorously vetting anything proposed as a utility class, keeping as much as possible associated with specific components. Too many utilities tends to lead to wide-scale scope creep and a lack of focus in a design system.

Another thought: I don't love that the demonstrated use cases showcase the image being squashed and stretched, and it's making me wonder if this is how we really want aspect ratios to be implemented.

Usually images are provided from design already in their desired aspect ratio, and manipulating their width/height metrics via CSS independently (i.e. not locked to one another) is not desired. What are we actually trying to achieve with this new functionality? It seems like a container with an aspect ratio (often implemented in CSS via padding-top percentages) and some form of cover/contain functionality will probably get us closer to our desired end goal.

[pastelcyborg]

Thanks for making these changes. I'm wondering if we should change the examples to make it a bit easier to tell what each of them is actually doing; because the image used is so small and there's no background color or border, it's kind of difficult to understand the aspect ratio being applied, since the examples all kind of just look like a big white frame. Even just making the example embed smaller (particularly horizontally) might help here.

[jmuzina]

@bartaz @pastelcyborg I have made the requested change to use is-highlighted for the image container with background, per our chat

[jmuzina]

I'm wondering if we should change the examples to make it a bit easier to tell what each of them is actually doing; because the image used is so small and there's no background color or border, it's kind of difficult to understand the aspect ratio being applied, since the examples all kind of just look like a big white frame.

@pastelcyborg I will revise this so the aspect ratio examples use an image with a background or border. The background highlight image will remain the same however as that background-less image is the intended use case for the background highlight.

[jmuzina]

@bartaz

I think it makes more sense to use the example with is-highlighted background, so that you actually see the container having aspect ratio, with image centered within. So I guess we could reuse the example from the is-highlighted, just add aspect ratio to it.

Revised accordingly:

[pastelcyborg]

This all looks good to me at this point, unless @bartaz has any additional thoughts.

[bartaz]

Small tweaks to release notes

[bartaz]

That reminded me that we actually have a feature to generate list of component class names from the code comment, like this: https://vanillaframework.io/docs/base/code#class-reference

Maybe it would be relatively simple to add something like that to image container? It's basically a yaml at the beginning of the scss file:
https://github.com/canonical/vanilla-framework/blob/main/scss/_patterns_code-snippet.scss#L1-L45

[jmuzina]

Revised to use class reference for classname documentation

[bartaz]

Let's not talk about the deprecated ones, less stuff to remove later.

[bartaz]

Thanks for all the updates and discussions, all looks good!