Fix: Converted inline item width to CSS variable (fixes #158)

[guywillis]

Fixes: #158

Fix

• Converted inline item width to CSS variable

[swashbuck]

I guess my issue with the CSS variable approach is that it makes the following difficult:

In a custom theme, if we then needed the items to only be 30% width instead of 33%, we would simply add it to the theme:

.has-3-columns .hotgrid__item {
   width: 30%;
}

This would also make it easier to adjust the width at different breakpoints as we can rely on @media queries in the Less.

@media (min-width: 10rem) and (max-width: 20rem) {
  .has-3-columns .hotgrid__item {
     width: 23%;
  }
}

[oliverfoster]

Could you explain how you propose to fix this part of my previous comment:

You'd have to specify has-x-columns classes for all of the available x options

Such that all of the correct width values are present with predefined classes:

.has-2-columns .hotgrid__item {
   width: 50%;
}

.has-3-columns .hotgrid__item {
   width: 33%;
}

.has-4-columns .hotgrid__item {
   width: 25%;
}

.has-5-columns .hotgrid__item {
   width: 20%;
}

Would you use a mixin and add all of the possible classes to the less output (2-10 columns?), would you have to manually add more classes if you needed more columns?

Note: We added configuration classes like this a lot on previous components and menus (narrative, cover, blinds, etc) and it's very brittle and limited and irritating to write and understand and change and always adds excess less that isn't used to every project. Width values should be calculated at runtime rather than being predefined, it's easier and simpler.

When adding the class has-x-columns to the component element, a component with has-3-columns would need to switch to has-2-columns and back at the small-medium transition, as columns always collapse to 2/50% width on mobile.

You could add the .has-${_columns}-columns class to the component element, without predefining the class widths, and carry on using the css variables to calculate the widths. This way it would be be possible to calculate the widths at runtime and/or override specific column settings using classes only.

It sounds as though you'd like to change all x column hotgrids in one override whereas the CSS variable implementation is just to switch inline defined widths to class-based widths, which are easier to override without an !important directive.

[swashbuck]

You'd have to specify has-x-columns classes for all of the available x options

@oliverfoster This doesn't concern me that much as we can always specify "up to" a reasonable amount of options and use a mixin to create the classes. Unused styles are also unavoidable in many cases, so it doesn't bother me to have unused column classes hanging around (like the Vanilla theme's column classes).

Width values should be calculated at runtime rather than being predefined, it's easier and simpler.

I think this is an issue of separating styling from presentation/code. Style values should all live in the theme if possible.

It sounds as though you'd like to change all x column hotgrids in one override whereas the CSS variable implementation is just to switch inline defined widths to class-based widths, which are easier to override without an !important directive.

Yes, classes are much easier to override and changes would most likely be at a global level.

Also, I haven't looked at your recent commit(s). But as long as we have something like has-x-columns as a class, I can work with that for any custom theme work.

[swashbuck]

👍

[kirsty-hames]

Works great thanks 👍 minor comment added regarding trailing space

[github-actions]

🎉 This PR is included in version 4.6.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀