canonical / vanilla-framework

From community websites to web applications, this CSS framework will help you achieve a consistent look and feel.
https://vanillaframework.io
GNU Lesser General Public License v3.0
830 stars 165 forks source link

Hero Pattern: Build Hero 50-50 HOC #5189

Closed jmuzina closed 2 months ago

jmuzina commented 3 months ago

Done

This does not build documentation for the hero pattern's new layouts; it merely implements the new layouts. Documentation will be completed as part of a separate PR to avoid blowing up scope of the initial implementation.

Fixes WD-13474

QA

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:

Screenshots

image image

webteam-app commented 3 months ago

Demo

Jenkins

demos.haus

jmuzina commented 3 months ago

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

bartaz commented 3 months ago

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 commented 3 months ago

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
lyubomir-popov commented 3 months ago
  • Is this functionality needed?

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

jmuzina commented 3 months ago

Blocked until merge of CTA block component (#5196 )

lyubomir-popov commented 2 months ago

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

jmuzina commented 2 months ago

@lyubomir-popov Addressing your feedback:

bartaz commented 2 months ago

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:

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?

jmuzina commented 2 months ago

@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 commented 2 months ago

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

lyubomir-popov commented 2 months ago

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 commented 2 months ago

@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.