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: Quote Wrapper Pattern #5275

Merged
merged 5 commits into from
Oct 23, 2024
Merged

feat: Quote Wrapper Pattern #5275

merged 5 commits into from
Oct 23, 2024

Conversation

jmuzina
Copy link
Member

@jmuzina jmuzina commented Aug 6, 2024

Done

  • Implements Quote Wrapper pattern per design and spec
  • Drive-bys:
    • Adds notes to all patterns docs pages about the patterns being a recent/in-flux addition

Fixes WD-13881

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:

  • 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 release (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.

Screenshots

Screenshot 2024-09-27 at 2 13 58 PM Screenshot 2024-09-27 at 2 14 07 PM Screenshot 2024-09-27 at 2 14 23 PM

@jmuzina jmuzina added the Feature 🎁 New feature or request label Aug 6, 2024
@jmuzina jmuzina self-assigned this Aug 6, 2024
@webteam-app
Copy link

@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch from fef001b to aa0f4e2 Compare August 13, 2024 18:29
@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch from 0ff810b to cb2d5c4 Compare August 13, 2024 18:39
@jmuzina jmuzina changed the title wip: feat: Quote Wrapper Pattern feat: Quote Wrapper Pattern Aug 13, 2024
@jmuzina jmuzina marked this pull request as ready for review August 13, 2024 18:53
@jmuzina jmuzina requested a review from pastelcyborg August 13, 2024 18:56
Copy link
Contributor

@pastelcyborg pastelcyborg left a comment

Choose a reason for hiding this comment

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

There appears to be something going on here with the width of the hr/divider - at some screen sizes, it doesn't stretch all the way to horizontally align with the image below:

image

templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/_macros/quote-wrapper.jinja Outdated Show resolved Hide resolved
templates/docs/patterns/quote-wrapper/index.md Outdated Show resolved Hide resolved
templates/docs/patterns/quote-wrapper/index.md Outdated Show resolved Hide resolved
@jmuzina jmuzina marked this pull request as draft August 13, 2024 20:02
@jmuzina
Copy link
Member Author

jmuzina commented Aug 13, 2024

There appears to be something going on here with the width of the hr/divider - at some screen sizes, it doesn't stretch all the way

@pastelcyborg Good catch, I think I've fixed that by moving the highlighted rule to outside of the grid, so it covers the full width of the whole pattern

@jmuzina jmuzina marked this pull request as ready for review August 13, 2024 20:40
@jmuzina jmuzina requested a review from pastelcyborg August 13, 2024 20:40
@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch from b6756c3 to 9cf0d37 Compare August 14, 2024 14:25
@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch 2 times, most recently from 672f139 to 04c7128 Compare September 27, 2024 18:08
@jmuzina
Copy link
Member Author

jmuzina commented Sep 27, 2024

Made some changes after meeting with @lyubomir-popov to discuss c.com PR 1380.

  • Design change: Changed the medium layout design: it now uses a 33/66 split with the signpost (or a gap if no signpost is provided) on the left, rest of the content on the right. The header's grid was also adjusted to use 33/66 on medium to line up with the signpost/quote grid.
  • Wrapping the quote text and the signpost logo in a shallow section
  • Slightly reduced spacing between the header row and rest of the quote
  • Fixed some broken grid usage for the signpost logo
  • Switched from using base hr to p-rule component
  • Show the HR above the quote text on all breakpoints instead of only on large

@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch 3 times, most recently from e4b2da7 to aee5372 Compare October 9, 2024 00:48
@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch from d85a05b to fe483ee Compare October 11, 2024 17:46
@jmuzina jmuzina marked this pull request as ready for review October 11, 2024 17:48
@jmuzina jmuzina force-pushed the hoc-quote-wrapper-macro branch from fe483ee to a4df366 Compare October 11, 2024 17:49
@jmuzina
Copy link
Member Author

jmuzina commented Oct 11, 2024

Spec has been approved, putting this PR back into consideration.

Updated this to follow the same naming and documentation standards we set with hero and tiered list, and bumped the version to 4.18.0.

Review when you can @bartaz @advl - hoping we can merge this next week :)

@jmuzina jmuzina requested a review from bartaz October 11, 2024 17:50
@jmuzina jmuzina added the Priority: High Should be addressed within current iteration label Oct 11, 2024
@bartaz
Copy link
Member

bartaz commented Oct 23, 2024

I guess it could be worth updating the pattern to use new 8 col grid? But also, doesn't need to be in scope of this PR. We could address it as follow up (could be done for all patterns I suppose).

@bartaz bartaz added Review: Percy +1 and removed Review: Percy needed This PR needs a review of Percy for visual regressions labels Oct 23, 2024
@bartaz bartaz removed their assignment Oct 23, 2024
Copy link
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Looks good. We could consider updating this (and other?) patterns to new grid, but this can be a follow up.

@jmuzina jmuzina merged commit 20522a6 into main Oct 23, 2024
13 checks passed
@jmuzina jmuzina deleted the hoc-quote-wrapper-macro branch October 23, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants