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

Improve title "block" so it works with and without editor styles #6909

Merged
merged 3 commits into from May 30, 2018

Conversation

Projects
None yet
3 participants
@jasmussen
Contributor

jasmussen commented May 23, 2018

The title block is tricky. Because it's not a block. It also relies on a textarea as opposed to a div with contenteditable. Because of this, the unique rules that apply to textareas make it hard to style with negative margins and such.

It's very complicated, but the short of it is, in order to look and behave like a block, the CSS styles are somewhat complex. I recently introduced a regression as a result of trying to make it more theme style friendly. But the fix to that regression caused a regression in turn to the theme styles.

This PR tries to fix all that. The title now looks and behaves like a block, on all breakpoints, and it's easily stylable by theme editor styles. For example, with the following CSS rules, you can have an editor that fills the entire canvas:

body.gutenberg-editor-page {

	.editor-post-title__block,
	.editor-default-block-appender,
	.editor-block-list__block,
	.editor-block-list__block[data-align="wide"],
	.editor-block-list__block[data-align="full"] {
		max-width: none;
	}

}

Or you can even apply a specific pixel or percent value.

Previously you had to make special rules JUST for the title block, so as to match the other elements.

One less cool thing about this PR, and suggestions are welcome, is that it introduces a new div, and renames a CSS class (to follow bem rules of hierarchy). I wish there was a different way.

Thoughts?

Screenshots:

screen shot 2018-05-23 at 10 34 39

screen shot 2018-05-23 at 11 08 08

@jasmussen jasmussen self-assigned this May 23, 2018

@jasmussen jasmussen requested review from mcsf and youknowriad May 23, 2018

@jasmussen jasmussen requested a review from WordPress/gutenberg-core May 30, 2018

@gziolo gziolo added this to the 3.0 milestone May 30, 2018

@tofumatt

This all looks good to me and I think making the title more like other blocks is 👍

Just some comment/code quality cleanups requested from me; once they're done this is good to merge so I say approved.

position: absolute;
top: -35px;
top: -$block-toolbar-height + 1px + 1px; // move upwards the toolbar height, but subtract borders

This comment has been minimized.

@tofumatt

tofumatt May 30, 2018

Member

This comment is a bit awkwardly-worded; how about:

top: -$block-toolbar-height + 1px + 1px; // Shift this element upward the same height as the block toolbar, minus the border size

All that said, add the 1px in there twice is weird. Why not just make a $border-width variable equal to 1px, then do -$block-toolbar-height + ($border-width * 2)? It'd be easier to read.

color: $dark-gray-900;
// stack borders
> div {
margin-left: -1px;

This comment has been minimized.

@tofumatt

tofumatt May 30, 2018

Member

All these 1px values are, I'm guessing, border-related as well? I definitely think it'd be worth having a $border-size or $border-width variable in that case.

This comment has been minimized.

@jasmussen

jasmussen May 30, 2018

Contributor

Good point. Considering #6773 which does major surgery on variables, I'd like to do that separately. Addressing the other bit now, thanks!

jasmussen added some commits May 23, 2018

Improve title "block" so it works with and without editor styles
The title block is tricky. Because it's not a block. It also relies on a `textarea` as opposed to a div with `contenteditable`. Because of this, the unique rules that apply to textareas make it hard to style with negative margins and such.

It's very complicated, but the short of it is, in order to _look and behave_ like a block, the CSS styles are somewhat complex. I recently introduced a regression as a result of trying to make it more theme style friendly. But the fix to that regression caused a regression in turn to the theme styles.

This PR tries to fix all that. The title now looks and behaves like a block, on all breakpoints, _and_ it's easily stylable by theme editor styles. For example, with the following CSS rules, you can have an editor that fills the entire canvas:

```
body.gutenberg-editor-page {

	.editor-post-title__block,
	.editor-default-block-appender,
	.editor-block-list__block,
	.editor-block-list__block[data-align="wide"],
	.editor-block-list__block[data-align="full"] {
		max-width: none;
	}

}
```

Or you can even apply a specific pixel or percent value.

Previously you had to make special rules JUST for the title block, so as to match the other elements.

One less cool thing about this PR, and suggestions are welcome, is that it introduces a new div, and renames a CSS class (to follow bem rules of hierarchy). I wish there was a different way.

Thoughts?
@jasmussen

This comment has been minimized.

Contributor

jasmussen commented May 30, 2018

@tofumatt Addressed your feedback, but also fixed an issue with the title block in the html editor. Can you take another look?

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented May 30, 2018

Got approval in a DM, and it's a small change after the initial approval, so merging.

@jasmussen jasmussen merged commit 0f620fc into master May 30, 2018

2 checks passed

codecov/project 46.46% remains the same compared to 29092ae
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasmussen jasmussen deleted the fix/editor-width-rules branch May 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment