WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#46611 closed defect (bug) (fixed)

Mobile menu icon in admin not displaying properly in 5.2-alpha

Reported by: earnjam Owned by: desrosj
Milestone: 5.2 Priority: normal
Severity: normal Version: 5.2
Component: Administration Keywords: has-patch commit
Focuses: ui Cc:

Description

As noted by @afercia in Slack, the small screen menu icon in the admin is not loading properly in trunk. See attached screenshot.

Likely related to [44940]

Attachments (5)

image.png (24.5 KB) - added by earnjam 3 months ago.
46611.diff (416 bytes) - added by oztaser 3 months ago.
46611.2.diff (66.5 KB) - added by oztaser 3 months ago.
fonts.zip (132.7 KB) - added by desrosj 3 months ago.
46611.3.diff (279.2 KB) - added by desrosj 3 months ago.

Download all attachments as: .zip

Change History (15)

@earnjam
3 months ago

@oztaser
3 months ago

#1 @desrosj
3 months ago

  • Component changed from General to Administration

Thanks for the patch @oztaser. Dashicons is managed externally in a repo on GitHub.

The font was recently updated to be generated with a new build process. Instead of changing the character in this stylesheet, we need to investigate why this character was removed from the font, and it needs to be added back for backwards compatibility.

I can work on this Monday, but feel free to investigate this before then if you’d like. Pinging @joen.

#2 @oztaser
3 months ago

@desrosj thanks for the feedback. I realized the icon unicode is not the same with auto-generated one. That's why the character looks like removed from the font.
I changed codepoints for the icon and created a merge request. Hopefully, I am not missing something else and these changes work.

https://github.com/WordPress/dashicons/pull/349

@oztaser
3 months ago

#3 @Joen
3 months ago

PR looks good, thanks @oztaser. That will fix this issue.

But it also helped uncover a few potential other issues, including email-alt2. Let's look at those separately. Let's coordinate in https://github.com/WordPress/dashicons/pull/349#issuecomment-476090036, would appreciate any help.

#4 @Joen
3 months ago

I have opened https://github.com/WordPress/dashicons/pull/350 to address additional issues discovered as a result of this ticket.

Please help test that PR!

#5 @desrosj
3 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

@desrosj
3 months ago

@desrosj
3 months ago

#6 @desrosj
3 months ago

  • Keywords has-patch needs-testing added

46611.3.diff is a patch for the issue described above, and a few more backwards compatibility fixes:

  • buddicons-bbpress-logo (f12b) was moved back to the correct location of f477
  • editor-ltr (f129) was moved back to the correct location of f10c
  • editor-ltr and editor-rtl had been switched. The arrows now point in the correct directions.
  • email-alt2 (f10a) was moved to the correct location of f467.

Also, these icons are restored:

  • menu-alt (f228)
  • camera-alt (f129)
  • edit-large (f327)
  • editor-distractionfree (f211)
  • update-alt (f113)
  • twitter-alt (f302)
  • text-page (f121)
  • plugins-checked (f485)
  • menu-alt3 (f349)
  • menu-alt2 (f329)
  • Duplicate lock icon at location (f315)
  • Duplicate editor-code icon at location (f494)

Also, this icon is being added:

  • code-standards (f13a)
Last edited 3 months ago by desrosj (previous) (diff)

#7 @oztaser
3 months ago

Hi @desrosj. I tested your patch and everything looks good to me.

There is also a new icon added. I think we need duplicate one for backwards compatibility. Am I right?

arrow-up-duplicate (f143)

#8 @ianbelanger
3 months ago

  • Keywords needs-testing removed

All looks good on Windows Chrome, Firefox, IE11 and Edge. @oztaser the arrow-up-duplicate icon is there, @desrosj just forgot it on his list

#9 @desrosj
3 months ago

  • Keywords commit added

@oztaser @ianbelanger Thanks for testing! I missed that one, thanks for pointing it out.

The arrow up icon was previously included in the font twice, but was removed from the f143 location in [44940].

#10 @desrosj
3 months ago

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

In 45040:

Administration: Fix Dashicon backwards compatibility issues.

In [44940], the Dashicon files in core were updated to contain the latest version of the font from the GitHub repo. This change follows that up by fixing several backwards compatibility issues caused by the new build process used to generate the font files and fixes the missing menu icon when viewing the admin on small screens.

  • buddicons-bbpress-logo (f12b) was moved back to the correct location of f477.
  • editor-ltr (f129) was moved back to the correct location of f10c.
  • email-alt2 (f10a) was moved to the correct location of f467.

The following icons were restored to their previous locations:

  • camera-alt (f129)
  • edit-large (f327)
  • editor-distractionfree (f211)
  • update-alt (f113)
  • twitter-alt (f302)
  • text-page (f121)
  • plugins-checked (f485)
  • menu-alt3 (f349)
  • menu-alt2 (f329)
  • menu-alt (f228)
  • Duplicate lock icon at location f315
  • Duplicate editor-code icon at location f494
  • arrow-up at f143.

The editor-ltr and editor-rtl icons had also been switched. The arrows now point in the correct directions at the correct Unicode locations (f10c and f320 respectively).

And, lastly, this change also introduces the code-standards (f13a) icon that was not included in the previous build.

Props afercia, earnjam, oztaser, joen, cathibosco1, ianbelanger, desrosj.
See #41074.
Fixes #46611.

Note: See TracTickets for help on using tickets.