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

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

Merged
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 kjellr added the [Type] Enhancement A suggestion for improvement. label Jul 2, 2020
@kjellr kjellr requested a review from jasmussen July 2, 2020 17:47
@kjellr kjellr self-assigned this Jul 2, 2020
@github-actions
Copy link

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

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

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 commented Jul 6, 2020

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
@kjellr kjellr deleted the try/better-line-height-for-post-title branch July 6, 2020 12:22
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jul 6, 2020
@alexvornoffice
Copy link

I think we should leave as it was, now the theme devs must update their CSS styling rules...

@kjellr
Copy link
Contributor Author

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
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants