Make WordPress Core

Opened 4 years ago

Last modified 7 months ago

#45950 new defect (bug)

Twenty Nineteen: Fix social media icons to use the correct height & width attribute

Reported by: crunnells's profile crunnells Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0.3
Component: Bundled Theme Keywords: has-screenshots good-first-bug has-patch
Focuses: Cc:

Description

This comes from a PR in GitHub: https://github.com/WordPress/twentynineteen/pull/754

The PR in question proposes to change social icon sizes to 26px, because of what _seems_ to be an erroneous setting in twentynineteen_nav_menu_social_icons(). I suggest removing the 26, since the CSS is set to show the icons at 32px, which overrides the 26px _anyway_.

Maybe update the default from 24 to 32 as well?

Attachments (3)

24px SVG icon.png (45.9 KB) - added by mukesh27 4 years ago.
32px SVG icon.png (46.8 KB) - added by mukesh27 4 years ago.
45950.diff (802 bytes) - added by tahmidulkarim 8 months ago.

Download all attachments as: .zip

Change History (11)

#1 @mukesh27
4 years ago

@crunnells Yes theme use css of 32 height and width of SVG icon and icon render 26 social and link icon with 24.

#2 @mukesh27
4 years ago

  • Component changed from General to Bundled Theme

#3 @laurelfulford
4 years ago

  • Keywords 2nd-opinion added

Thanks @mukesh27 and @crunnells!

Just to make sure: @allancole or @kjellr, was the smaller size (24 or 26) meant to override anything in the social menu? As Chris pointed out, the CSS is setting the actual size (32); I agree that switching it to 32 in the code as well makes sense if that's the intended display size. I just want to make sure I'm not missing something here - thanks!

#4 @laurelfulford
4 years ago

  • Keywords has-screenshots needs-patch added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release

Got confirmation that this can be switched to 32 in twentynineteen_nav_menu_social_icons(), so the code matches how the icons actually display.

#5 @desrosj
8 months ago

  • Keywords good-first-bug added

#6 @tahmidulkarim
8 months ago

  • Keywords has-patch added; needs-patch removed

I have changed it to 32. Patch attached.

@tahmidulkarim
8 months ago

This ticket was mentioned in PR #3668 on WordPress/wordpress-develop by dkeech.


7 months ago
#7

I updated the instances of svg icons in icon-functions.php to 26px from 24px.

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

This ticket was mentioned in PR #3668 on WordPress/wordpress-develop by dkeech.


7 months ago
#8

I updated the instances of svg icons in icon-functions.php to 26px from 24px.

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

Note: See TracTickets for help on using tickets.