Add sections and blocks components

[yurii-vasyliev]

Done

Added three new classes: .p-block, .p-section for managing spacing between different types of elements on the page.

Fixes WD-3019

QA

• Open demo
• Review updated examples:
  • blocks
  • sections
• Review updated documentation:
  • sections and blocks
• Check if brochure site docs and examples use new section/block components:
  • https://vanilla-framework-4730.demos.haus/docs/layouts/brochure
  • https://vanilla-framework-4730.demos.haus/docs/examples/layouts/brochure-site/index

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.

[webteam-app]

Demo starting at https://vanilla-framework-4730.demos.haus

[bartaz]

@yurii-vasyliev Could you separate removing your previous utils to separate branch? This one will need design review, so may need more time to be merged, and I don't want to block release on having the utils in main branch. I'd like to be able to release whatever else we have.

[lyubomir-popov]

How are we handling the case where people start aplying this to headings and override the baseline grid alignment?

[bartaz]

How are we handling the case where people start aplying this to headings and override the baseline grid alignment?

We are not, they are not supposed to do that. It's a container element, not text element.

If people start to put class names on wrong elements we can't support whatever comes out of it. The best we can do is properly document and provide examples.

By principle you are NEVER supposed to put multiple component class names on single element. So if section/block is a component you don't put this on a heading. But on a wrapper element around the content.
That's why these are components, not utilities. You are supposed to use them with correct markup, not just throw a class name on random element of your choice.

[lyubomir-popov]

@bartaz We have a separate issue where @anthonydillon said the spacing should be on the text - can't remember which one it was. People will definitely do it, as much as we don't want them to. We need to make sure it works as expected IMO. We all know how overused the u-sv utilities are.

[bartaz]

@bartaz We have a separate issue where @anthonydillon said the spacing should be on the text - can't remember which one it was. People will definitely do it, as much as we don't want them to. We need to make sure it works as expected IMO. We all know how overused the u-sv utilities are.

Components are not supposed to work on any kind of element. And that's exactly why I think spacing should work on a component level. This way we can avoid issues that u-sv utility creates. The fact that "people do it" is caused by the fact that we gave them this utility. We have a chance to make it better.

If we provide components that have clearly defined scope, document how to use them (and how NOT to use them) I think we can assume developers will be using them correctly and avoid adding loads of exceptions to Vanilla that give an impression that you can use this component like an utility on anything you want (while in reality you can't).

If we add exceptions for all heading levels, paragraphs, then what is next? Should it work when you use it with any other component? Lists, chips, grid, navigation? There is a reason why components are independent and are not supposed to be mixed with each other on single element.

We talked about it and it seems we have exact and clear rules about when to use each kind of spacing:

• sections of the website have regular spacing or deep if they are highlighted
• sibling blocks within a section have a smaller sibling spacing

These rules can be neatly implemented into components with clear scope and use: you have a top level section on the page? use section (deep if it's highlighted), you have multiple blocks within the section: use block container for them.

Allowing to put spacing on any element on the page makes these rules fuzzy and harder to apply. I'm trying to make the Vanilla components implement the design rules. And as far as I understand in design terms we talk about sections and siblings, not about adding spacing to arbitrary headings and paragraphs on the page.

And if I'm wrong, than I guess we need to get back to drawing board with that…

[lyubomir-popov]

@bartaz sorry I think I got confused - I think I saw u-sv--shallow, u-sv--deep, etc - that's what I was talking about. Looking at the code now it is p-section, and I fully agree with you about those cases - noone will put that on text. Did the pr change? Or did I see u-sv-sdeep in some other part and leave the comment in the wrong place

[bartaz]

@bartaz sorry I think I got confused - I think I saw u-sv--shallow, u-sv--deep, etc - that's what I was talking about. Looking at the code now it is p-section, and I fully agree with you about those cases - noone will put that on text. Did the pr change? Or did I see u-sv-sdeep in some other part and leave the comment in the wrong place

You are right, it used to be implemented as u-sv--shallow kind of utilities, but after issues with had with these, and discussion with you about how this spacing should work it was rewritten into components. So yes, the PR changed, and I guess you commented without realising this. No worries.

Proposed change is not needed

[bartaz]

From talking with Lyubo it seems that the "deep" sections will always need a top and bottom padding, so should be implemented using existing strips.

The new section component will only be used instead of strip (or inside the strip) when section follow each other without background change or emphasis.

So the --deep variant can be removed.

[ClementChaumel]

What is the !important for?

[bartaz]

I think it was a leftover from utilities that were used before, should be removed.

[ClementChaumel]

I just noticed that the margin in the example is not quite right (I added an outline for clarity)

They are spaced more than they should because the <p>'s margin-bottom is bigger than the block's

[lyubomir-popov]

overflow:auto fixes the above ^

@bartaz Separately, if I wanted to make this section dark or light grey, how would I do it? Asking as the spac eat the bottom is done with a margin, so that part would not be tinted by any background applied to the p-section or any of its children.

[ClementChaumel]

overflow:auto fixes the above ^

@bartaz Separately, if I wanted to make this section dark or light grey, how would I do it? Asking as the spac eat the bottom is done with a margin, so that part would not be tinted by any background applied to the p-section or any of its children.

According to the docs you shouldn't do it! 🙈

[lyubomir-popov]

@ClementChaumel ah sorry I forgot. Still think padding-bottom would be more predictable and intuitive given strips use padding-bottom

[ClementChaumel]

I thought that maybe instead we could remove the margin bottom from the last element?

I added some more content to visualise it better:

Here it is now with the big spacing:

And here it is with the margin of the last <p> removed

Which one do you reckon looks best?

[lyubomir-popov]

@ClementChaumel I've often thought of doign this but refrained as it might backfire. Let's try it and see what happens. We can always drop it if it causes issues. Now's the time to experiment with things like this.

[bartaz]

@ClementChaumel I've often thought of doign this but refrained as it might backfire. Let's try it and see what happens. We can always drop it if it causes issues. Now's the time to experiment with things like this.

@ClementChaumel @lyubomir-popov the issue unfortunately is also with how to implement it. We can't just margin-bottom: 0 to last child, because (unfortunately) it would mess up baseline grid spacing. We would need to apply u-no-margin--bottom styles to any last element, not knowing what type it is, etc. I don't see a reliable way to do it right now without rethinking how bottom spacing is applied.

We also not always know if something is last child - there may be additional wrappers around things, like columns.

[lyubomir-popov]

the issue unfortunately is also with how to implement it.

let's not do it then. we don't want to make it more complicated.

[ClementChaumel]

@ClementChaumel @lyubomir-popov the issue unfortunately is also with how to implement it. We can't just margin-bottom: 0 to last child, because (unfortunately) it would mess up baseline grid spacing. We would need to apply u-no-margin--bottom styles to any last element, not knowing what type it is, etc. I don't see a reliable way to do it right now without rethinking how bottom spacing is applied.

What do you mean, what happens if we just set it to 0?

[lyubomir-popov]

@ClementChaumel for example, a p's margin-bottom is 1.5rem - $nudge (.4rem). That's what keeps it on the baseline grid, and the nudge value differs at every font-size. So if you replace 1.1rem with 0, you go off the baseline grid.

[ClementChaumel]

@ClementChaumel for example, a p's margin-bottom is 1.5rem - $nudge (.4rem). That's what keeps it on the baseline grid, and the nudge value differs at every font-size. So if you replace 1.1rem with 0, you go off the baseline grid.

Ah ok I see what you mean! I personally find it less noticeable than the consistent spacing but that's fine if that's what we want to prioritise.
And as Bartek mentioned, the implementation is non trivial anyway