Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rich horizontal list): Rich horizontal list pattern #5456

Merged
merged 10 commits into from
Feb 6, 2025

Conversation

jmuzina
Copy link
Member

@jmuzina jmuzina commented Jan 23, 2025

Done

  • Implements rich horizontal list pattern / HOC per spec and design
  • Drive-by: Fixed some language inconsistencies in the rich vertical list docs (such as interchangeably referring to it as "vertical rich list" and "rich vertical list", and referencing the Hero in the import instructions)

Fixes WD-17706

QA

Check if PR is ready for release

If this PR contains Vanilla SCSS or macro 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 existing APIs (CSS classes & macro APIs) are not changed it can be a bugfix release (x.x.X)
    • if existing APIs (CSS classes & macro APIs) are changed/added/removed it should be a minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) or macros should be listed on the what's new page.

Screenshots

image

@jmuzina jmuzina added the Feature 🎁 New feature or request label Jan 23, 2025
@webteam-app
Copy link

@jmuzina
Copy link
Member Author

jmuzina commented Jan 23, 2025

@lyubomir-popov I've completed an initial draft of this pattern's implementation.

Per a discussion we had a few days ago, I want to draw your attention to one difference from the Figma design. We built the horizontal section component with intrinsic container queries to demo those. These container queries are not (nor should they be) aligned with the screen-level responsive breakpoints, so the horizontal section list changes column counts at different breakpoints than the rest of the pattern. This results in a small range where there are four list columns but the CTA has been moved to the first column due to reaching tablet size

Screencast.from.2025-01-23.16-31-04.webm

Also, the logo section breaks out of the 50/50 split on tablet size, before the column counts have changed:

Screencast.from.2025-01-23.16-32-16.webm

I would add that this isn't really a "bug", rather it is a feature of intrinsic sizing as components are sized independently from eachother based on their contents. It is just a bit interesting to see this behavior when we combine elements built with container and media queries.

Is this acceptable? If not, we can make an adjustment to the horizontal section so that it uses the responsive grid instead of container queries.

@lyubomir-popov
Copy link
Contributor

Is this acceptable?

Absolutely. That's the beauty of container queries. Nice work!

@jmuzina jmuzina force-pushed the WD-17706-ft-rich-horizontal-list-hoc branch 3 times, most recently from c1b45ab to 9d1729d Compare January 24, 2025 17:41
@jmuzina jmuzina changed the title wip: feat(rich horizontal list): Rich horizontal list pattern feat(rich horizontal list): Rich horizontal list pattern Jan 24, 2025
@jmuzina jmuzina force-pushed the WD-17706-ft-rich-horizontal-list-hoc branch 3 times, most recently from e61fd33 to e73cc7a Compare January 24, 2025 18:11
@jmuzina jmuzina marked this pull request as ready for review January 24, 2025 18:57
@lyubomir-popov
Copy link
Contributor

Left one comment on figma, the h2 needs a p-section--shallow wrapper. all good apart from that, nice work!

@jmuzina jmuzina force-pushed the WD-17706-ft-rich-horizontal-list-hoc branch from 0dfe9ba to a073e0c Compare January 27, 2025 18:03
advl
advl previously approved these changes Jan 29, 2025
@jmuzina
Copy link
Member Author

jmuzina commented Jan 30, 2025

A code reviewer from the sites team is being assigned, per @petesfrench

@britneywwc britneywwc self-assigned this Feb 3, 2025
@jmuzina jmuzina requested a review from britneywwc February 4, 2025 22:10
@jmuzina
Copy link
Member Author

jmuzina commented Feb 5, 2025

Thanks for the reviews @britneywwc ! I've addressed all of your comments, could you have another look please?

Copy link
Contributor

@britneywwc britneywwc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes @jmuzina, looks great! :)

@jmuzina jmuzina merged commit b790038 into main Feb 6, 2025
12 checks passed
@jmuzina jmuzina deleted the WD-17706-ft-rich-horizontal-list-hoc branch February 6, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature 🎁 New feature or request Review: Code +1 Review: Design +1 Review: Percy needed This PR needs a review of Percy for visual regressions Review: QA +1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants