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.

Sign up for GitHub

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

Jump to bottom

Refine padding/margin, override block gap #28

Merged
merged 4 commits into from
Sep 29, 2021
Merged

Refine padding/margin, override block gap #28

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ebb98e
Theme: Refine padding and margins.
iandunn Sep 21, 2021
9ba3fc9
fix post nav margin
iandunn Sep 27, 2021
0431f0f
remove unused breakpoints
iandunn Sep 29, 2021
3e0d2bd
Merge branch 'trunk' into head-foot-styles
iandunn Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions bs-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ module.exports = {
paths.globalHeaderFooter + watchedFilesPattern
],

"ignore": [
paths.theme + '/sass/**/*.*',
paths.globalHeaderFooter + '/postcss/**/*.*'
],

"open": false,
"reloadOnRestart": true,
"notify": false,
Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "wporg-news-2021-project",
"version": "1.0.0",
"description": "",
"description": "The codebase and development environment for w.org/news.",
"author": "WordPress.org",
"license": "GPL-2.0-or-later",
"private": true,
Expand All @@ -24,6 +24,7 @@
"sync": "browser-sync start --config bs-config.js"
},
"workspaces": [
"source/wp-content/themes/wporg-news-2021"
"source/wp-content/themes/wporg-news-2021",
"source/wp-content/mu-plugins/wporg-mu-plugins"
]
}
1 change: 1 addition & 0 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ These must be run in the project's root folder, _not_ in theme/plugin subfolders

* Build all assets once: `yarn workspaces run build`
* Rebuild all assets on change: `yarn workspaces run start`
* Reload browser when files change: `npm run sync`
5 changes: 3 additions & 2 deletions source/wp-content/themes/wporg-news-2021/block-template-parts/post-navigation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
<hr class="wp-block-separator alignfull has-text-color has-background has-light-grey-background-color has-light-grey-color is-style-wide"/>
<!-- /wp:separator -->

<!-- wp:group {"tagName":"nav","align":"wide","className":"post-navigation"} -->
<nav class="wp-block-group alignwide post-navigation">
<!-- wp:group {"tagName":"nav","align":"full","className":"post-navigation"} -->
<nav class="wp-block-group alignfull post-navigation">
Comment on lines -5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change this to full? Won't this make it so the previous/next links are out on the edges of the viewport instead of staying within the content column?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 , good point. IIRC I did it so that the content would line up w/ the footer content, but i'll take a closer look.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, i think this works fine. we could also change it to work w/ alignwide, but i think this makes more sense semantically, since post navigation isn't part of the content of a post

(at least it's not in the DOM; philosophically i can see it either way, but would personally learn towards it being outside)

Copy link
Member Author

Choose a reason for hiding this comment

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

I went ahead and merged to keep the # active branches low, but I'm happy to continue discussing if you have any other thoughts

Copy link
Contributor

Choose a reason for hiding this comment

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

Just checked this on a super wide screen, and it looks like it does make the previous/next links sit too far out. My read on the design mockup is that the "previous" link should line up with the post date and author on the left side, and the "next" link should be justified right and line up with the edge of the post title and content column on the right side.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see what you're saying.

I wonder if it should be aligned with the post content and the header/footer, and that the real problem here is that there isn't a "max width" of sorts on the header/footer. I'm guessing we don't want those to grow endlessly.

@beafialho can you add a Figma example of what a whole page should look like when the browser is 2000px wide?

Copy link
Collaborator

@beafialho beafialho Sep 30, 2021 •
edited by iandunn
Loading

Choose a reason for hiding this comment

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

Sure, here's what it would look like in a 2000px browser. The behaviour should be: post navigation aligns with the edges of the grid. The header is not included in this grid, but the footer is. Does it make sense?

Captura de ecrã 2021-09-30, às 16 37 24

Captura de ecrã 2021-09-30, às 16 37 03

Prototype
Mockup

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks!

moved discussion to #37 since this pr is already merged

<!-- wp:group {"className":"post-navigation-previous"} -->
<div class="wp-block-group post-navigation-previous">
<!-- wp:post-navigation-link {"type":"previous","showTitle":true,"label":"Previous"} /-->
</div>
<!-- /wp:group -->

<!-- wp:group {"className":"post-navigation-next"} -->
<div class="wp-block-group post-navigation-next">
<!-- wp:post-navigation-link {"type":"next","showTitle":true,"label":"Next"} /-->
Expand Down
4 changes: 2 additions & 2 deletions source/wp-content/themes/wporg-news-2021/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "wporg-news-2021",
"name": "wporg-news-2021-theme",
"version": "1.0.0",
"description": "",
"description": "A Full Site Editing theme for w.org/news, with a jazz-inspired design.",
"author": "WordPress.org",
"license": "GPL-2.0-or-later",
"private": true,
Expand Down
32 changes: 25 additions & 7 deletions source/wp-content/themes/wporg-news-2021/sass/base/_alignment.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
//FRONTEND
.wp-site-blocks { // top level of the view
//In this situation we want to introduce a standardized gap between content and the edge of the screen.
padding-left: var(--wp--custom--post-content--padding--left);
.site-content-container {
// In this situation we want to introduce a standardized gap between content and the edge of the screen.
padding-top: var(--wp--custom--post-content--padding--top);
padding-right: var(--wp--custom--post-content--padding--right);
padding-bottom: var(--wp--custom--post-content--padding--bottom);
padding-left: var(--wp--custom--post-content--padding--left);

.alignfull {
// these elements we want to "bust out" of the gap created above
// These elements we want to "bust out" of the gap created above
margin-left: calc(-1 * var(--wp--custom--post-content--padding--left)) !important;
margin-right: calc(-1 * var(--wp--custom--post-content--padding--right)) !important;
width: unset;
// however any containers that "bust out" should re-apply that gap but this time using padding instead of margins.

// However any containers that "bust out" should re-apply that gap but this time using padding instead of margins.
&.wp-block-template-part,
&.wp-block-columns,
&.wp-block-group {
Expand All @@ -18,12 +22,26 @@
}
}

@include break-tablet {
.site-content-container {
padding-top: var(--wp--custom--post-content--breakpoint--tablet--padding--top);
padding-right: var(--wp--custom--post-content--breakpoint--tablet--padding--right);
padding-bottom: var(--wp--custom--post-content--breakpoint--tablet--padding--bottom);
padding-left: var(--wp--custom--post-content--breakpoint--tablet--padding--left);

.alignfull {
margin-left: calc(-1 * var(--wp--custom--post-content--breakpoint--tablet--padding--left)) !important;
margin-right: calc(-1 * var(--wp--custom--post-content--breakpoint--tablet--padding--right)) !important;
}
}
}

// EDITOR (NOTE: It PROBABLY would be OK to bring these together to "simplify" the stylesheet. However the selectors are quite different
// and it's a lot easier to understand and ensure intent separated in this way.
.is-root-container { //top level of the editor
.is-root-container { // Top level of the editor.
padding-left: var(--wp--custom--post-content--padding--left);
padding-right: var(--wp--custom--post-content--padding--right);
.wp-block[data-align="full"] { //blocks configured to be "align full" in "editorspeak"
.wp-block[data-align="full"] { // Blocks configured to be "align full" in "editorspeak".
margin-left: calc(-1 * var(--wp--custom--post-content--padding--left)) !important;
margin-right: calc(-1 * var(--wp--custom--post-content--padding--right)) !important;
width: unset;
Expand Down
19 changes: 19 additions & 0 deletions source/wp-content/themes/wporg-news-2021/sass/base/_breakpoints.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,22 @@
@content;
}
}

// Custom breakpoints should be synced with `global-header-footer` in `wporg-mu-plugins`.
@mixin break-tablet() {
@media (min-width: 876px) {
@content;
}
}

@mixin break-desktop() {
@media (min-width: 1152px) {
@content;
}
}

@mixin break-desktop-wide() {
@media (min-width: 1320px) {
@content;
}
}
18 changes: 0 additions & 18 deletions source/wp-content/themes/wporg-news-2021/sass/base/_header.scss
View file
Open in desktop

This file was deleted.

6 changes: 6 additions & 0 deletions source/wp-content/themes/wporg-news-2021/sass/base/_headings.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

h1, h2, h3, h4, h5, h6 {
&.has-background {
padding: var(--wp--custom--padding--vertical) var(--wp--custom--padding--horizontal);
}
}
8 changes: 5 additions & 3 deletions source/wp-content/themes/wporg-news-2021/sass/base/_layout.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
min-height: 100vh;
display: flex;
flex-direction: column;
}

.site-footer-container {
margin-top: auto;
/*
* Make sure the footer is always at the bottom of the viewport, even on pages with little content.
* Otherwise there'd be empty space below the footer
*/
justify-content: space-between;
}

body.admin-bar {
Expand Down
2 changes: 1 addition & 1 deletion source/wp-content/themes/wporg-news-2021/sass/base/_style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
@import "breakpoints";

@import "alignment";
@import "header";
@import "headings";
@import "layout";
@import "text";
@import "utility";
Empty file added source/wp-content/themes/wporg-news-2021/sass/blocks/_navigation.scss
View file
Open in desktop
Empty file.
2 changes: 0 additions & 2 deletions source/wp-content/themes/wporg-news-2021/sass/blocks/_paragraph.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
p {


// Override `color: inherit` from Core styles.
&.has-text-color a {
color: var( --wp--style--color--link, var(--wp--custom--color--primary) );
Expand Down
16 changes: 16 additions & 0 deletions source/wp-content/themes/wporg-news-2021/sass/post/_navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
[class*="wp-container-"] > .post-navigation {
padding-top: 0;
padding-right: var(--wp--custom--post-content--padding--right);
padding-bottom: 19px;
padding-left: var(--wp--custom--post-content--padding--left);
}

@include break-tablet {
[class*="wp-container-"] > .post-navigation {
padding-top: 0;
padding-right: 88px;
padding-bottom: 26px;
padding-left: var(--wp--custom--post-content--breakpoint--tablet--padding--left);
}
}

.post-navigation {
display: flex;
justify-content: space-between;
Expand Down
1 change: 1 addition & 0 deletions source/wp-content/themes/wporg-news-2021/sass/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ GNU General Public License for more details.
@import "blocks/image";
@import "blocks/latest-posts";
@import "blocks/list";
@import "blocks/navigation";
@import "blocks/paragraph";
@import "blocks/post-author";
@import "blocks/post-comments";
Expand Down
20 changes: 18 additions & 2 deletions source/wp-content/themes/wporg-news-2021/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
"//README": "Comments should be added to document important things. Use the `//{property}` format. See https://github.com/WordPress/gutenberg/issues/35099.",
"$schema": "https://json.schemastore.org/theme-v1.json",
"version": 1,
"settings": {
"border": {
Expand Down Expand Up @@ -338,8 +340,20 @@
},
"post-content": {
"padding": {
"left": "20px",
"right": "20px"
"top": "32px",
"bottom": "70px",
"left": "var(--wp--style--block-gap)",
"right": "var(--wp--style--block-gap)"
},
"breakpoint": {
"tablet": {
"padding": {
"top": "80px",
"bottom": "80px",
"left": "80px",
"right": "180px"
}
}
}
},
"pullquote": {
Expand Down Expand Up @@ -395,6 +409,8 @@
"wideSize": "var(--wp--custom--layout--wide-size)"
},
"spacing": {
"//blockGap": "The design calls for a 24px gap. Coincidentally, that's the default value that Gutenberg defines. The default could change in the future, though, so this should be explicitly set.",
"blockGap": "24px",
"customPadding": true,
"units": [
"%",
Expand Down