Update Media Object to support new color theme variables

[pastelcyborg]

Done

• Updated Media Object component to support new color theme variable

Please note the code comment at the top of the file: we can no longer use the vf-url-friendly-color function for theming the icons, as it cannot interpolate CSS variables during compilation (since they only exist at runtime). Instead, I hard-coded different inline icons with specific colors for light/dark themes (paper will use the light theme colors). I think this works, particularly given this element is intended to be deprecated anyway.

Fixes WD-11867

QA

• Open default, large, and circle demos
• Ensure component looks as expected across all themes

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 release (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.

[webteam-app]

Demo

Jenkins

demos.haus

[jmuzina]

Nice approach! I think this is fine since this component is deprecated as you said.

[bartaz]

Yes, that's the easiest way to deal with legacy components not supporting the CSS variables.

I'd just add support for having is-dark/is-light class name on itself as well:

.is-dark &,
&.is-dark

And similar with is-light to make sure we can override it to light if needed.

[pastelcyborg]

I'm not sure I totally follow, @bartaz - you're saying to add the is-dark theme class onto the p-media-object__meta-list-item itself? What would be the scenario in which someone would need to add the theme class directly onto this element?

[bartaz]

Yes, the theme class names can be put on any level. So as a simplification for such legacy components I think it's good to at least support 2 "extremes" (setting class on body - which should be the new most common way to do it, or setting it on component itself - which was the old way to do it).

We do it in similar way in icons:

vanilla-framework/scss/_patterns_icons.scss

Lines 373 to 377 in 70b856e

	body.is-dark &,
	&.is-light, // DEPRECATED: use is-dark instead
	&.is-dark {
	@include vf-icon-applications($colors--dark-theme--icon);
	}

This way, we should be always able to override the parent theme, by setting theme on the component.
For example if there is a mix of is-dark and is-light on parent elements (because page is dark, but then there is a light strip, and in this light strip there is media object), if the media object thinks it should be "dark" (because of dark class on body), but is actually on light background, we can set is-light on it directly, to override it.

[pastelcyborg]

Sorry, I guess I'm still misunderstanding.

The way I have it written, a descendent selector is used to implement themed icons:

.is-dark .p-media-object__meta-list-item--date

If I add an additional selector in the way I'm interpreting your comment, this wouldn't really add any useful functionality:

.p-media-object__meta-list-item---date.is-dark

Because there's no reason to add the theme class at this child level. My descendent selector would already handle both the cases of the theme class existing at the body level or at the component (.p-media-object or similar) level.

[bartaz]

Ah, sorry, my bad. They way I mentioned is supposed to be on component root element level.

You know what, it's a legacy component anyway, likely not used in any dark theme situations. So your approach should be fine. In case we spot a bug with that we will address it properly (likely by updating those icons, or redesigning that).

[bartaz]

LGTM, sorry for the confusion.