vanilla-framework icon indicating copy to clipboard operation
vanilla-framework copied to clipboard

Add our p-section--shallow spacing to h1,h2s by default

Open lyubomir-popov opened this issue 1 year ago • 4 comments

Wrapping headings in p-section--shallow is one of the most comment QA comments I have to leave, which leads me to think there's room for improvement. My propoosal:

  • add it by default on h1, h2
  • provide a class that is added on a parent, and allows us to reduce that margin-bottom in places like documentation sites, where we get consistent requests to make things significantly denser. We can then further expand this to affect font-sizes, also a requirement from the documentation team

Before: image image

After: image image

lyubomir-popov avatar Jul 29 '24 09:07 lyubomir-popov

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/WD-13715.

This message was autogenerated

The quick way to do this is to extend the shallow section spacing placeholder in the heading 1 placeholder (which is extended by the heading 2 placeholder)

%vf-heading-1 {
    // same as %section-padding--shallow
    padding-bottom: $spv--x-large;
    // ..

However, this would be a breaking change for users who implement this spacing in the current way. They will end up with double the spacing (padding added to the section and to the header).

<div class="p-section--shallow">
  <h1>Some header text</h1>
   <!--Bottom padding here-->
</div>
<!--Bottom padding here-->

I suppose a workaround could be to create a padding collapse override for headings that are direct children of a shallow section, but I'm not sure how I feel about targeting a component class inside of a base style mixin.

%vf-heading-1 {
  // same as %section-padding--shallow
  padding-bottom: $spv--x-large;

  .p-section--shallow > & {
    padding-bottom: 0;
  }
}

@pastelcyborg @bartaz any thoughts?

Also @lyubomir-popov , should anything be done with the responsive spacing added to the headings? https://github.com/canonical/vanilla-framework/blob/327bc2e6cbb0801be76fb8feb3b53e416996f659/scss/_base_typography-definitions.scss#L25-L30

jmuzina avatar Sep 11 '24 19:09 jmuzina

Possibly unpopular opinion: if the tools already exist in Vanilla to achieve the desired design, I would leave this as-is for now. Yes, we could probably very slightly optimize this, but I would counter with two sentiments:

  1. We often see design requesting modifications/changes via rounds of iteration; I think we should resist the urge to knee-jerk make sweeping code changes to accommodate what could be a fleeting request
  2. We have a lot of other large-scale work to complete, so if the current implementation and features aren't actually broken or prohibitive to completing sites, this should be a very low priority

pastelcyborg avatar Sep 13 '24 18:09 pastelcyborg

Triage: that is high effort, and quite high impact. That's probably something to consider for new architecture.

We seem to consistently struggle with this. It seems that we need different typography styles/rules for different contexts (websites, documentation, apps, …other?). This has to be properly defined in the design system.

bartaz avatar Sep 25 '24 07:09 bartaz