Twenty Nineteen Button is having issue with line height.

[nidhidhandhukiya]

Steps to reproduce the issue :-

1. Activate Twenty Nineteen theme.
2. Choose Button block.
3. Add some text.
4. Apply line height.

You can able to see when you apply line height it will add space on above the button.

I have attached video for better unnderstanding.
Video URL :- ​https://share.cleanshot.com/Sy247vzrHM6gWH69y5D1

[monzuralam]

@nidhidhandhukiya, Thanks for your ticket.

We are looking for testing report. More than one test report is a helpful way to validate across different environments.

[shailu25]

@monzuralam

Test Report for - https://core.trac.wordpress.org/attachment/ticket/58443/58443.patch Patch.

Environment:
============
WordPress - V6.2.2
Theme - Twenty Nineteen
Os: Windows
Browser: Chrome

Backend:
========
Before Patch Backend: ​https://prnt.sc/VoB3ZIbiqwZO
After Patch Backend : ​https://prnt.sc/FIOdY2k8XG-q

Frontend:
=========
Before Patch Frontend: ​https://prnt.sc/plXeZ-xUnuRt
After Patch Frontend : ​https://prnt.sc/UZacP-yMFEKm

https://core.trac.wordpress.org/attachment/ticket/58443/58443.patch patch is working fine.

[hrrarya]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58443/58443.patch

Environment

• OS: macOS 12.6.7
• Web Server: Nginx
• PHP: 7.4.33
• WordPress: 6.3-beta3-56130-src
• Browser: Brave - Version 1.52.129 Chromium: 114.0.5735.198 (Official Build) (x86_64)
• Theme: Twenty Nineteen
• Active Plugins: - No Active Plugins

Actual Results

• ✅ Issue resolved with patch.

Supplemental Artifacts

Before patch backend - ​https://prnt.sc/MXqw7vmabIZb
After patch backend - ​https://prnt.sc/EzQ4gft0tHZZ

Before patch frontend - ​https://prnt.sc/M_UeEnkMFhZN
After patch frontend - ​https://prnt.sc/MYSuzufmPrSo

[karmatosed]

I am going to commit this as can see it fixed with the patch, thank you.

[karmatosed]

In 56580:

Twenty Nineteen: Fixes button line height.

Line height was broken for button block and adding spacing.

Props nidhidhandhukiya, shailu25.
Fixes #58443.

[karmatosed]

In 56581:

Twenty Ninteen: Revert changes to 'style-rtl.css'.

This revert changes to Twenty Nineteen 'style-rtl.css' file made in [56580] as it auto-generated during the build process.

Unprops karmatosed.
Props costdev.
See #58443.

[karmatosed]

On the basis of consulting regarding some failed tests this needs a little more of a solid fix at the SASS file level. Just fixing the CSS means that tests fail for example for the rtl files which shouldn't be changed.

@nidhidhandhukiya I wanted to give you the opportunity to work on this patch considering you originally did but happy to also tackle this if not.

cc @costdev and @spacedmonkey for those who aided diagnosing why the tests were not passing regarding this issue.

[sabernhardt]

Using Switching a specific value to inherit was not the best way to fix custom line-height.
Correction: When the theme—or block library styles—specify the desired value on a parent element, then using inherit could be highly appropriate.

On the front end, ​all button-like elements have had a line-height of 1.2 since ​initial commit, and it needs to stay that value when users do not select their own. This ticket did not change the front-end style.css and the RTL was reverted, so the current state is still only wrong with a custom line-height.

The editor's value of 1.8 did not match the front, but removing the entire line can be better than changing to inherit.

Any Some of the customizable properties probably could be moved to the wp-block-button or wp-block-buttons element instead of the link.
Correction: The em-based font-size is one property that probably should not be moved to another element because that would change the size for anyone who has set a value in custom CSS or who is satisfied with the relative difference in size using the block setting.

If tests are still giving any errors, that might be related to the missing space after the colon in style-editor.scss.

[harshgajipara]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58443/58443.1.patch

Environment

OS: Windows
PHP: 8.1.9
WordPress: 6.3.1
Browser: Chrome
Theme: Twenty Nineteen
Plugins: None activated

Actual Results:

Before Patch:
✅ In the editor and frontend, space showing above button when line height applied.
Backend: ​https://prnt.sc/lQl2hEgLCwi8
Frontend: ​https://prnt.sc/Jeo1j1zraYLL

After Patch
✅ In the editor and frontend, space removed above button when line height applied.
Backend: ​https://prnt.sc/WtJaVDtD0I7m
Frontend: ​https://prnt.sc/9CH_svNz2k2H

[nicolefurlan]

@nidhidhandhukiya thank you for reporting and working on this! Would you be able to review #comment:13 and make changes to the patch?

I moved line-height default definition from .wp-block-button__link to .wp-block-button, as a result, it allowed for overriding by style and block editor.

Trac ticket: https://core.trac.wordpress.org/ticket/58443

[fnpen]

@nicolefurlan, I made changes according to the #comment:13.

I moved line-height default definition from .wp-block-buttonlink to .wp-block-button, as a result, it allowed for overriding by style and block editor.

Trac ticket: [)](https://core.trac.wordpress.org/ticket/58443 [core.trac.wordpress.org/ticket/58443]

[nicolefurlan]

Pinging previous testers to see if they can test the updated patch :)

cc @shailu25, @hrrarya, @harshgajipara

[nicolefurlan]

@poena pinging you to see if you could rally come folks to test this one out so it could potentially make it into RC1 next week :)

[shailu25]

Test Report

Patch Tested: ​https://github.com/WordPress/wordpress-develop/pull/5431

Environment:

WordPress - 6.4 beta2
OS - Windows
Browser - Chrome
Theme: Twenty Nineteen
PHP - 8.1.2
Active Plugin - None

Actual Results:

• After applying the patch line height issue will be resolved.✅

Screenshots:

Back-End: ​https://prnt.sc/ZDqvaBTbW_Lr

Front-End:
Before Patch Frontend : ​https://prnt.sc/VIB-qf5LC_8b
After Patch Frontend : ​https://prnt.sc/qOjDtEb6rO1X

It is very late in this release cycle for revising style-editor.scss too, but could you set line-height: $font__line-height-heading; on .wp-block-button and remove the line-height from .wp-block-button__link there for consistency?

[sabernhardt]

I needed to edit two statements in #comment:13 because they were not entirely accurate.

Thanks for the newer patch. PR 5431 is an improvement but not thorough. It adds support for the line height setting on the front end when applied to individual Button blocks, yet not when trying to set a line height on the parent Buttons block. Whether that is good enough or should be applied to .wp-block-buttons instead, I would like at least the same change in the theme's editor styles.

For quicker testing, you could copy and paste a ​set of Button blocks. The first set focuses on the default typography styles and custom line height, and the second set includes several other custom typography options (Twenty Nineteen still would not support a custom font weight or text decoration).

[nicolefurlan]

@fnpen would you be able to update your PR?

I'm going to bump this to 6.5 since this patch needs changes and it will need further testing.

Moves line-height to Buttons container and removes the duplicate outline

Trac 58443

[poena]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/6069.

Environment

• OS: macOS 14.2.1
• Web Server: Nginx
• PHP: 8.1.23
• WordPress: 6.5-alpha-56966-src
• Browser: Chrome ersion 121.0.6167.160

Actual Results

• ✅ Issue resolved with patch.

Additional Notes

• The default line height is correct in both the editor and front.
• The highlight can be adjusted both on the buttons block and button block:

For example the line height on the buttons can be 2.5 while an individual button inside it can have a line height set to 0. Both line heights are correct in the editor and front.

[huzaifaalmesbah]

Test Report

Patch tested: ​PR 6069

Environment

• WordPress: 6.5-alpha-56966-src
• PHP: 8.2.15
• Server: nginx/1.25.3
• Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
• Browser: Chrome 121.0.0.0
• OS: macOS
• Theme: Twenty Nineteen 2.7
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.1.0

Actual Results

Line height issue resolved with patch ✅

I restarted the jobs, since the Theme ZIP Build test for Twenty Nineteen was failing

This test is still failing

[audrasjb]

@poena @sabernhardt looks like the Theme ZIP Build test for Twenty Nineteen is failing

Do I understand correctly from the error report that the rtl css files need to be re-built from the scss files and added to the PR?

Yes, the RTL stylesheet needed to be built.

[SergeyBiryukov]

In 57599:

Twenty Nineteen: Correct line height for Button block.

This commit moves line-height to the Buttons container and removes the duplicate outline property.

Props sabernhardt, nidhidhandhukiya, monzuralam, shailu25, hrrarya, karmatosed, harshgajipara, nicolefurlan, fnpen, oglekler, poena, huzaifaalmesbah, audrasjb.
Fixes #58443.

Thanks for the PR! Merged in r57599.

Thanks for the PR! Merged in r57599.

Restores transition property to a single line, as its was before r57599.

Trac 58443

[sabernhardt]

reopening to fix tests

[SergeyBiryukov]

In 57604:

Twenty Nineteen: Restore transition property to a single line.

Follow-up to [57599].

Props sabernhardt.
Fixes #58443.

Thanks for the PR! Merged in r57604.