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

wip: Hero pattern: new HOC layouts

Open jmuzina opened this issue 1 year ago • 6 comments

Done

Builds examples of the Hero HOC per Design

Fixes WD-12214

QA

(these are WIP and may be subject to change / PR splitting as this progresses)

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:

  • [x] PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
  • [x] 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
  • [x] Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

image

jmuzina avatar Jun 25 '24 17:06 jmuzina

I've temporarily made the CTA block display: inline-block using an inline style to prevent it from wrapping the link's text rather than wrapping the entire link, like this:

Before: image

After: image

  1. Is this functionality needed?
  2. If so, is there some class in Vanilla as-is that I can use to achieve this rather than using inline styles?

Here's an example: https://vanilla-framework-5189.demos.haus/docs/examples/patterns/hero/hoc-50-50

@bartaz @pastelcyborg @lyubomir-popov

jmuzina avatar Jun 25 '24 20:06 jmuzina

While we did call those "higher order components" at the time of planning, and it's still mentioned in some places, we want to call those "patterns" in the design system. So let's avoid the "hoc" in documentation, file names, etc.

bartaz avatar Jun 26 '24 08:06 bartaz

We already have a bunch of existing examples for hero pattern, so I hoped this will be just making sure that we cover what is needed, and possibly don't need to add a bunch of completely new ones.

image

bartaz avatar Jun 26 '24 08:06 bartaz

  • Is this functionality needed?

yes that's what it should do, wrap as entire block.

lyubomir-popov avatar Jun 26 '24 16:06 lyubomir-popov

Blocked until merge of CTA block component (#5196 )

jmuzina avatar Jun 26 '24 20:06 jmuzina

generally they look really good! A list of things I've spotted:

  • what does "is-shallow" do? image

  • can we use the imagfe aspect ratio classes? this one should be 2x3 for example: image

  • these should be p-section--shallow. Generally, inside a component we use shallow, the regular p-section is for spacing entire sections on the page: image image

  • we need to bring these two close together, as when they are siblings. not sure how exactly as they are in different containers. this will likely be a thorny thing to solve image

  • can we spell out "paragraph" as it looks strange when it wraps: image

  • padding bottom should be p-section, not shallow - this is the bottom of the entire section/component: image

  • the answer here should probably be a hard "NO", but just in case :) any chance we could add an option to stretch the image to the full height, when content is longer? We loose the aspect ratio, but it could be useful I think: image

  • if this is done with macros, how do we hide images? it's a commonly used technique for large decorative images on small screens. Not something for this pr probably, as there are no macros here, but we shjould be able to pass 1 or 2 u-hide--{insert-breakpoint here} classes to the image container.

lyubomir-popov avatar Jul 17 '24 17:07 lyubomir-popov

@lyubomir-popov Addressing your feedback:

  • [x] .is-shallow: this was unnecessary and not actually doing anything - think I stole it from p-strip. It's been removed
  • [x] Image aspect ratio classes: Added!
  • [x] Paragraphs should be wrapped in .p-section--shallow: Updated!
  • [ ] H1/H2 should be closer together in 50-50-full-cover-image: I attempted to address this by removing the p-section--shallow around the H1 and adding a u-no-margin-bottom to the H1, let me know what you think.
  • [x] Spell out "paragraph"
  • [ ] Bottom padding should be the padding for p-section, not shallow: Are you referring to the bottom padding of .p-section and .p-section--hero (which is set to $spv--strip-regular)? If so, what would you like that changed to?
  • [ ] Stretch image to full height: I suspect this may be out of scope of this PR, maybe a modification of the image container instead. Is it a must-have?
  • [x] Macros: Implemented with macro, including option to pass in list of breakpoints to hide the hero image on.

jmuzina avatar Jul 17 '24 17:07 jmuzina

I feel like we have too many options and examples here, it's getting hard to understand what are the differences between those.

If the main first distinction is going to be a layout, so let's maybe focus on 50/50 only first.

What I'm missing from Figma design is a better understanding of what are the different variations and how they behave responsively.

My understanding currently is:

  • 50/50
    • text content on left, image on right (optional)
    • text content on top (h1 on left, rest on the right), image on the bottom (optional)
  • 75/25
    • text content on left, image on right (optional)
  • 25/75
    • (optional) signpost image on left, text content on right, (optional) image on bottom

Anything else seems to me a content difference (aspect ratio of the image, some optional content being used or not). There are no other different layout options, or am I missing something?

bartaz avatar Jul 18 '24 12:07 bartaz

@lyubomir-popov Can you please re-org the Hero Figma design in a similar way to the split list? In SU today we were having a hard time differentiating the different variants & breakpoints.

Also, can you let us know which variants should "stack" on medium (use -on-large) or not?

jmuzina avatar Jul 18 '24 14:07 jmuzina

Per SU today, this PR is being split to only tackle the 50/50 variants & add macro support.

jmuzina avatar Jul 18 '24 14:07 jmuzina

one more - can we please change all images that span the full grid width to --cinematic aspect ratio. 16/9 makes it too tall

lyubomir-popov avatar Jul 18 '24 16:07 lyubomir-popov

@lyubomir-popov Can you please re-org the Hero Figma design in a similar way to the split list? In SU today we were having a hard time differentiating the different variants & breakpoints.

yes here https://www.figma.com/design/Hll6ZRvHyTTGBZZSoRd8ei/Higher-order-components-(HOCs)?node-id=1584-5346&t=Rs4MS0K3bl2zhzIg-1

they all have the exact same medium/small, I haven't added a split version on medium for simplicity. It's already complicated enough.

lyubomir-popov avatar Jul 18 '24 16:07 lyubomir-popov