Skip to content

/ gutenberg

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

Jump to bottom

Try: Add line height rule to the post title #23656

Merged
merged 1 commit into from Jul 6, 2020
Merged

Try: Add line height rule to the post title #23656

merged 1 commit into from Jul 6, 2020

Conversation

@kjellr
Copy link
Contributor

@kjellr kjellr commented Jul 2, 2020

When using Gutenberg's default editor styles, the line height of the post title seems really tall to me. The post title mimics the font size of the h1 block, but it does not set its own line height. This PR tries syncing up the line height with the h1 block.

The post title did have a line-height rule, but it was set to inherit, under a Inherit the styles set by the theme. comment. If a theme sets its own line height, it'll be overridden here anyway. If a theme does not set its own line height, it's quite likely that at this font size, a line height of 1.4 will be more appropriate than whatever the body line height is set to anyway. I tried with a number of themes (Twenty Nineteen, Twenty Twenty, Go, etc), and this seemed to work just fine.

Before

Screen Shot 2020-07-02 at 1 37 44 PM

After

Screen Shot 2020-07-02 at 1 39 14 PM
@kjellr
Add line height to the post title.
Loading status checks…
6ff8b57
@kjellr kjellr requested a review from jasmussen Jul 2, 2020
@kjellr kjellr self-assigned this Jul 2, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Jul 2, 2020

Size Change: -2 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/editor/style-rtl.css 3.82 kB -1 B
build/editor/style.css 3.82 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.42 kB 0 B
build/block-directory/style-rtl.css 952 B 0 B
build/block-directory/style.css 951 B 0 B
build/block-editor/index.js 109 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.63 kB 0 B
build/block-library/editor.css 7.63 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 7.79 kB 0 B
build/block-library/style.css 7.79 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.65 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.88 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.57 kB 0 B
build/edit-post/style.css 5.56 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.02 kB 0 B
build/edit-site/style.css 3.02 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 44.8 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.73 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.3 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action
@jasmussen
jasmussen approved these changes Jul 3, 2020
View changes
Copy link
Contributor

@jasmussen jasmussen left a comment

If a theme sets its own line height, it'll be overridden here anyway.

Can you expand a little on this? I believe I may have been responsible for the initial change.

In the end, I don't have a strong opinion on this, and I frankly defer to you on the theme details as you're far more versed than me.

The intent, definitely, was for the post title to have line-height similar to that of other headings, but if that isn't the case, we might as well find a good balance in the interim, until such a time as the post title finally transforms into a butterfly block.
@kjellr
Copy link
Contributor Author

@kjellr kjellr commented Jul 6, 2020 •
edited

Can you expand a little on this? I believe I may have been responsible for the initial change.

Ah, I'm just saying that if a theme supplies a line height for the .editor-post-title__input, it'll override our default line height style here so the changes in this PR shouldn't cause any major conflicts.

The intent, definitely, was for the post title to have line-height similar to that of other headings, but if that isn't the case, we might as well find a good balance in the interim, until such a time as the post title finally transforms into a butterfly block.

Yep, that's right. After this PR, the post title has the same line height as the h1 heading block. 👍
@kjellr kjellr merged commit e6a7702 into master Jul 6, 2020
14 checks passed
14 checks passed
Check Check
Details
build
Details
Admin - 1
Details
pull-request-automation
Details
pull-request-automation
Details
test (gutenberg-editor-gallery)
Details
test (gutenberg-editor-gallery)
Details
All
Details
JavaScript
Details
Admin - 2
Details
PHP
Details
Admin - 3
Details
Mobile
Details
Admin - 4
Details
@kjellr kjellr deleted the try/better-line-height-for-post-title branch Jul 6, 2020
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jul 6, 2020
@alexvornoffice
Copy link

@alexvornoffice alexvornoffice commented Jul 9, 2020

I think we should leave as it was, now the theme devs must update their CSS styling rules...
@kjellr
Copy link
Contributor Author

@kjellr kjellr commented Jul 9, 2020

@alexvornoffice can you point me to a theme where this caused an issue? I tested across a handful of themes as described above, and didn't see any negative effects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasmussen jasmussen

Assignees

@kjellr kjellr

Labels
[Type] Enhancement
Projects
None yet
Milestone
Gutenberg 8.5
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@kjellr @alexvornoffice @jasmussen
You can’t perform that action at this time.