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

[River] Responsive spacing #236

Merged
merged 12 commits into from
May 15, 2023
Merged

Conversation

josepmartins
Copy link
Contributor

@josepmartins josepmartins commented Apr 18, 2023

Summary

New and updated spacing tokens on the River component aiming for a responsive logic in both gap and padding values of the main container

New padding values are:

Viewport Padding
Small (mobile) 28px -> total of 56px when used stacked
Medium (tablet) 36px -> total of 72px when used stacked
Large 48px -> total of 96px when used stacked

New gap values are:

Viewport Padding
Small (mobile) 24px
Medium (tablet) 32px
Large 48px

List of notable changes:

  • updated margin for small/medium/large-padding token
  • updated gap for small/medium/large-gap token
  • updated River.module CSS with new media queries values

What should reviewers focus on?

  • Storybook/docs with stacked River examples reacting to different viewports

Steps to test:

  1. Open the # component in CI-deployed preview environment
  2. Go to # story in Storybook
  3. Verify that # behaves as described in the following issue.

Supporting resources (related issues, external links, etc):

Contributor checklist:

  • All new and existing CI checks pass
  • Tests prove that the feature works and covers both happy and unhappy paths
  • Any drop in coverage, breaking changes or regressions have been documented above
  • New visual snapshots have been generated / updated for any UI changes
  • All developer debugging and non-functional logging has been removed
  • Related issues have been referenced in the PR description

Reviewer checklist:

  • Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
  • Check that tests prove the feature works and covers both happy and unhappy paths
  • Check that there aren't other open Pull Requests for the same update/change

Screenshots:

Please try to provide before and after screenshots or videos

Before After
actual.mp4
new.mp4

@changeset-bot
Copy link

changeset-bot bot commented Apr 18, 2023

🦋 Changeset detected

Latest commit: 9736c9c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@primer/brand-primitives Patch
@primer/react-brand Patch
@primer/brand-e2e Patch
@primer/brand-fonts Patch
@primer/brand-config Patch
@primer/brand-storybook Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@josepmartins josepmartins changed the title Add responsive padding values [River ] Responsive spacing Apr 18, 2023
@josepmartins josepmartins changed the title [River ] Responsive spacing [River] Responsive spacing Apr 18, 2023
@josepmartins josepmartins self-assigned this Apr 18, 2023
@josepmartins josepmartins added enhancement New feature or request update snapshots labels Apr 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 18, 2023

🔍 Design token changes found

View CSS variable changes
- --brand-River-layout-margin-vertical: var(--base-size-48) 0;
+ --brand-River-heading-margin: var(--base-size-8);

@josepmartins josepmartins force-pushed the josepmartins/river-responsive-values branch from 26b5f97 to df1c1ce Compare April 18, 2023 11:53
@github-actions
Copy link
Contributor

github-actions bot commented Apr 18, 2023

🟢 No visual differences found

Our visual comparison tests did not find any differences in the UI.

@josepmartins josepmartins temporarily deployed to github-pages April 18, 2023 12:01 — with GitHub Actions Inactive
@josepmartins josepmartins force-pushed the josepmartins/river-responsive-values branch from 96dbb4a to 4705df7 Compare April 18, 2023 13:57
@josepmartins josepmartins temporarily deployed to github-pages April 18, 2023 14:04 — with GitHub Actions Inactive
@josepmartins josepmartins removed update snapshots enhancement New feature or request labels Apr 19, 2023
@josepmartins josepmartins temporarily deployed to github-pages April 20, 2023 09:38 — with GitHub Actions Inactive
@josepmartins josepmartins temporarily deployed to github-pages April 27, 2023 13:08 — with GitHub Actions Inactive
@josepmartins josepmartins temporarily deployed to github-pages May 2, 2023 11:04 — with GitHub Actions Inactive
@josepmartins josepmartins temporarily deployed to github-pages May 2, 2023 14:19 — with GitHub Actions Inactive
Copy link
Collaborator

@rezrah rezrah left a comment

Choose a reason for hiding this comment

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

New spacing looks great 👍 :shipit:

@josepmartins josepmartins merged commit 7502285 into main May 15, 2023
@josepmartins josepmartins deleted the josepmartins/river-responsive-values branch May 15, 2023 13:46
@primer-css primer-css mentioned this pull request May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants