Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#46611 closed defect (bug) (fixed)

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

Reported by: earnjam's profile earnjam Owned by: desrosj's profile 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 6 years ago.
46611.diff (416 bytes) - added by oztaser 6 years ago.
46611.2.diff (66.5 KB) - added by oztaser 6 years ago.
fonts.zip (132.7 KB) - added by desrosj 6 years ago.
46611.3.diff (279.2 KB) - added by desrosj 6 years ago.

Download all attachments as: .zip

Change History (15)

@earnjam
6 years ago

@oztaser
6 years ago

#1 @desrosj
6 years 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
6 years 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
6 years ago

#3 @Joen
6 years 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
6 years 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
6 years ago

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

@desrosj
6 years ago

@desrosj
6 years ago

#6 @desrosj
6 years 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 6 years ago by desrosj (previous) (diff)

#7 @oztaser
6 years 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
6 years 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
6 years 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
6 years 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.