Make WordPress Core

Opened 8 weeks ago

Last modified 6 weeks ago

#61321 new enhancement

Twenty Twenty: consider improving Pullquote block padding

Reported by: hmbashar's profile hmbashar Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: needs-testing has-testing-info has-patch
Focuses: css Cc:

Description

Hi there,

I hope you're doing well!

I've been testing the "Pullquote" block in the Twenty Twenty theme and noticed something that could improve its appearance. When I add a border to the block, it becomes clear that adding some padding to the top and bottom would make it look much nicer.

Here are the steps to see what I mean:

  1. Activate the Twenty Twenty theme.
  2. Add a "Pullquote" block.
  3. Apply a border to the block and check out the layout.

I think this small change could make a big difference. Thanks for considering this improvement!

https://i.postimg.cc/ncjNfs95/Pullquote.png

Best Wishes
Bashar

Attachments (2)

Pullquote.png (15.4 KB) - added by hmbashar 8 weeks ago.
6741.diff (2.0 KB) - added by hmbashar 6 weeks ago.

Download all attachments as: .zip

Change History (17)

@hmbashar
8 weeks ago

#1 @poena
8 weeks ago

  • Type changed from defect (bug) to enhancement

#2 @sabernhardt
8 weeks ago

  • Focuses ui removed
  • Keywords changes-requested removed
  • Summary changed from Twenty Twenty: Suggestion to Improve "Pullquote" Block Padding in Twenty Twenty Theme to Twenty Twenty: consider improving Pullquote block padding when user adds a border
  • Version trunk deleted

This ticket was mentioned in PR #6684 on WordPress/wordpress-develop by @narenin.


8 weeks ago
#3

  • Keywords has-patch added; needs-patch removed

#4 follow-up: @narenin
8 weeks ago

Hi @hmbashar,

I am able to replicate the same issue at my end.

@sabernhardt @poena I have added the PR Fix for this.

#5 follow-up: @mdnesarmridha
8 weeks ago

  • Keywords needs-testing added; has-testing-info has-patch removed

I have added keywords according to ticket status

Last edited 8 weeks ago by mdnesarmridha (previous) (diff)

#6 in reply to: ↑ 5 @hmbashar
8 weeks ago

  • Keywords has-testing-info has-patch added

Replying to mdnesarmridha:

I have added keywords according to ticket status

Hi @mdnesarmridha,

I think you mistakenly removed a few keywords. I've added them back.
Thanks for your contribution.

I have updated the keywords according to the ticket status

#7 in reply to: ↑ 4 @hmbashar
8 weeks ago

Replying to narenin:

Hi @hmbashar,

I am able to replicate the same issue at my end.

@sabernhardt @poena I have added the PR Fix for this.

Thank you for replicating the issue and adding the PR fix. Your efforts are much appreciated!

#8 @huzaifaalmesbah
8 weeks ago

Thanks for the patch, @narenin I see that patch PR 6684 covers only the frontend. No changes have been made to the editor or RTL.

Last edited 8 weeks ago by huzaifaalmesbah (previous) (diff)

#9 @narenin
7 weeks ago

Hi @huzaifaalmesbah ,

@narenin I see that patch PR 6684 covers only the frontend. No changes have been made to the editor or RTL.

Thanks for the feedback, I have implemented the same, please check.

#10 @hmbashar
7 weeks ago

Test Report

Description

Hello @narenin,
I've tested the latest patch PR #6684, and it looks fine. However, I noticed a potential difference in padding between the backend and frontend, which might require some adjustments. Please review the attached screenshots.

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

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.3.7
  • Server: nginx/1.25.4
  • Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.7)
  • Browser: Chrome 125.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty 2.6
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. Issue resolved with patch but I think there might be a difference in padding between the backend and frontend and we may need to make some adjustments.

Additional Notes

  • Any additional details worth mentioning.

Before Apply Patch Screenshots

Editor Frontend
https://i.ibb.co/2Fjzp2f/Before-Editor.png https://i.ibb.co/sCyxhS9/Before-frontend.png

After Apply Patch Screenshots

Editor Frontend
https://i.ibb.co/xjMFB8N/After-Editor.png https://i.ibb.co/1fmpPJH/After-Frontend.png

#11 @narenin
7 weeks ago

@hmbashar Thanks for the detailed feedback.

I have added the PR with patch, could you please check now.

This ticket was mentioned in PR #6741 on WordPress/wordpress-develop by @hmbashar.


7 weeks ago
#12

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

https://github.com/WordPress/wordpress-develop/assets/7321834/ef6da2ce-5fde-4ecd-8cf6-df096dc3b946
https://github.com/WordPress/wordpress-develop/assets/7321834/e6f71abd-35ae-402a-a16c-c2e095fa559b
https://github.com/WordPress/wordpress-develop/assets/7321834/4224a5c6-e68e-41bd-823f-7e7dac92d0b3
https://github.com/WordPress/wordpress-develop/assets/7321834/e175c6da-29dd-4ed6-a4c1-09bb38609185

---
This Pull Request is for code review only.
Please keep all other discussions in the Trac ticket.
See GitHub Pull Requests for Code Review in the Core Handbook for more details.

#13 @hmbashar
7 weeks ago

  • Summary changed from Twenty Twenty: consider improving Pullquote block padding when user adds a border to Twenty Twenty: consider improving Pullquote block padding

Hello @narenin,

Thank you for your contribution. I believe that padding should always be present, not just when a border is used. When I tested with a background color and no border, it seemed that having padding would provide a better view. Therefore, I think padding should be available by default, regardless of whether a border or background is used. That's why I submitted a PR.

Again, thank you for your contribution.

Best Regards
Bashar

@rejaulalomkhan commented on PR #6741:


7 weeks ago
#14

Thanks For the excellent PR @hmbashar. This PR looks good for me.

@hmbashar
6 weeks ago

This ticket was mentioned in Slack in #accessibility by hmbashar. View the logs.


6 weeks ago

Note: See TracTickets for help on using tickets.