-
Notifications
You must be signed in to change notification settings - Fork 67
[Brand Refactor] Fixes SectionIntro layout #1360
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
Changes from 13 commits
12d8135
11325cf
849c270
2b765fb
b900823
c8b6d27
1535e09
5ff58ae
4daf8e9
5806ae4
b7b1969
5a24197
006c7c4
7c29d11
ad8dccc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@primer/react-brand': patch | ||
| --- | ||
|
|
||
| Updated `SectionIntroStacked` layout spacing for improved visual alignment. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,20 @@ | ||
| .SectionIntroStacked { | ||
| --brand-Grid-spacing-row: var(--base-size-48); | ||
| --brand-Grid-spacing-row: var(--base-size-64); | ||
| } | ||
|
|
||
| @media screen and (min-width: 48rem) and (max-width: 63.24rem) { | ||
| .SectionIntroStacked { | ||
| max-width: 618px; | ||
| margin-inline: auto; | ||
|
Comment on lines
+7
to
+8
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rezrah I ended up using a @jesussandreas, can you please confirm that this is the correct
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielguillan literally just seeing this. but it seems correct. |
||
| } | ||
|
|
||
| .SectionIntroStacked.SectionIntroStacked--variant-gridline { | ||
| padding-inline: var(--base-size-32); | ||
| padding-block: calc(var(--base-size-64) - var(--base-size-4)); | ||
|
|
||
| --SectionIntroStacked-item-divider-overflow-start: var(--base-size-32); | ||
| --SectionIntroStacked-item-divider-overflow-end: var(--base-size-32); | ||
| } | ||
|
danielguillan marked this conversation as resolved.
|
||
| } | ||
|
|
||
| .SectionIntroStacked--variant-gridline { | ||
|
|
@@ -21,15 +36,6 @@ | |
| } | ||
| } | ||
|
|
||
| @media screen and (min-width: 63.25rem) { | ||
| .SectionIntroStacked--variant-gridline { | ||
| padding-inline: var(--base-size-64); | ||
| padding-block: var(--base-size-64); | ||
| --SectionIntroStacked-item-divider-overflow-start: 0px; | ||
| --SectionIntroStacked-item-divider-overflow-end: calc(var(--brand-Grid-spacing-margin) + var(--base-size-64)); | ||
| } | ||
| } | ||
|
|
||
| @media screen and (min-width: 80rem) { | ||
| .SectionIntroStacked--variant-gridline { | ||
| border-inline: var(--brand-borderWidth-thin) solid var(--brand-color-border-muted); | ||
|
|
@@ -71,10 +77,13 @@ | |
| flex-direction: column; | ||
| } | ||
|
|
||
| @media screen and (max-width: 63.25rem) { | ||
| @media screen and (max-width: 63.24rem) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielguillan what was the reason for this breakpoint change? Want to understand the reason so we can consider fixing the inconsistencies in other components. We same to have more components using max-width of |
||
| .SectionIntroStacked-heading { | ||
| font-size: var(--brand-text-size-600); | ||
| } | ||
| .SectionIntroStacked-items { | ||
| overflow: hidden; | ||
| } | ||
|
danielguillan marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @media screen and (min-width: 63.25rem) { | ||
|
|
@@ -86,15 +95,18 @@ | |
| .SectionIntroStacked--variant-gridline .SectionIntroStacked-items { | ||
| border-inline-start: var(--brand-borderWidth-thin) solid var(--brand-color-border-muted); | ||
| margin-block: calc(-1 * var(--base-size-64)); | ||
| padding-block: 0; | ||
| padding-block: calc(var(--base-size-64) - var(--base-size-32)); | ||
| position: static; | ||
| top: auto; | ||
| } | ||
|
|
||
| .SectionIntroStacked-content { | ||
| padding-inline-end: var(--base-size-80); | ||
| } | ||
|
|
||
| .SectionIntroStacked--variant-gridline .SectionIntroStackedItem-item { | ||
| /* Inset item content from the vertical gridline */ | ||
| padding-inline-start: var(--base-size-32); | ||
| padding-inline-end: var(--base-size-32); | ||
| padding-inline-start: var(--base-size-64); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -137,26 +149,27 @@ | |
|
|
||
| .SectionIntroStackedItem-item--with-icon { | ||
| display: flex; | ||
| flex-direction: row; | ||
| align-items: flex-start; | ||
| gap: var(--base-size-24); | ||
| flex-direction: column; | ||
| } | ||
|
|
||
| @media screen and (min-width: 63.25rem) { | ||
| @media screen and (min-width: 48rem) { | ||
| .SectionIntroStackedItem-item--with-icon { | ||
| gap: var(--base-size-48); | ||
| flex-direction: row; | ||
| } | ||
| } | ||
|
|
||
| .SectionIntroStackedItem__content { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: var(--base-size-8); | ||
| gap: var(--base-size-12); | ||
| flex: 1; | ||
| min-width: 0; | ||
| } | ||
|
|
||
| .SectionIntroStackedItem__icon { | ||
| flex-shrink: 0; | ||
| border-radius: var(--brand-borderRadius-medium); | ||
| border-radius: var(--base-size-12); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielguillan is this intended for this PR? |
|
danielguillan marked this conversation as resolved.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielguillan @jesussandreas the space under the section intro is now really large on older templates like flex suite.. checkout this snapshot diff out for an example 👇. Personally feel the spacing is too much even on gridline, but it looks dramatically worse on older termplates. Could/should we constrain or reduce it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious whether we're accounting for some kind of inline borders here? Is that why it's not rounded up to 620px?