Make WordPress Core

Opened 4 months ago

Last modified 42 hours ago

#58443 assigned defect (bug)

Twenty Nineteen Button is having issue with line height.

Reported by: nidhidhandhukiya's profile nidhidhandhukiya Owned by:
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.2.2
Component: Bundled Theme Keywords: has-testing-info has-patch has-screenshots
Focuses: Cc:

Description

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

Attachments (2)

58443.patch (2.2 KB) - added by nidhidhandhukiya 4 months ago.
58443.1.patch (1.0 KB) - added by nidhidhandhukiya 13 days ago.

Download all attachments as: .zip

Change History (17)

This ticket was mentioned in Slack in #core-test by ironprogrammer. View the logs.


4 months ago

#2 @monzuralam
4 months ago

  • Keywords has-testing-info needs-testing has-patch needs-screenshots added

@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.

Last edited 4 months ago by monzuralam (previous) (diff)

#3 @monzuralam
4 months ago

  • Keywords needs-screenshots removed

#4 @monzuralam
4 months ago

  • Keywords has-screenshots added

#5 @shailu25
3 months ago

@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.

Last edited 3 months ago by shailu25 (previous) (diff)

#6 @hrrarya
3 months ago

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

#7 @karmatosed
2 weeks ago

  • Keywords commit added; needs-testing removed
  • Owner set to karmatosed
  • Status changed from new to assigned

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

#8 @karmatosed
2 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56580:

Twenty Nineteen: Fixes button line height.

Line height was broken for button block and adding spacing.

Props nidhidhandhukiya, shailu25.
Fixes #58443.

This ticket was mentioned in Slack in #core-committers by costdev. View the logs.


2 weeks ago

#10 @karmatosed
2 weeks ago

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.

#11 @karmatosed
2 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#12 @karmatosed
2 weeks ago

  • Owner karmatosed deleted
  • Status changed from reopened to assigned

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.

#13 @sabernhardt
2 weeks ago

  • Keywords commit removed

Using inherit was not the best way to fix custom line-height.

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 of the customizable properties probably could be moved to the wp-block-button or wp-block-buttons element instead of the link.

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

#14 @sabernhardt
9 days ago

  • Milestone changed from Awaiting Review to 6.4

#15 @harshgajipara
42 hours ago

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

Note: See TracTickets for help on using tickets.