WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#38488 closed defect (bug) (fixed)

Twenty Seventeen: Replace remaining Genericons with Font Awesome icons

Reported by: melchoyce Owned by: davidakennedy
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

I noticed all of the arrows are still using Genericons. Let's replace those with Font Awesome arrows.

We can replace the chevrons with FA's angle set.

The previous/next icons we'll want to experiment with to see which looks better: FA's arrows, long arrows, or carets.

Attachments (5)

38488.patch (26.8 KB) - added by sami.keijonen 4 years ago.
38488.1.patch (29.0 KB) - added by sami.keijonen 4 years ago.
38488.2.patch (30.9 KB) - added by laurelfulford 4 years ago.
38488.3.patch (28.7 KB) - added by davidakennedy 4 years ago.
Removes code from other patch.
38488.4.patch (29.4 KB) - added by laurelfulford 4 years ago.
Adds fix for missing SVG fallback in IE8.

Download all attachments as: .zip

Change History (30)

#1 follow-up: @sami.keijonen
4 years ago

Here are Genericons used and why.

  • expand: no similar icon in FA.
  • next: no similar icon in FA but plenty of arrows to chooce.
  • previous: no similar icon in FA but plenty of arrows to chooce.
  • pinned: no similar icon in FA.
  • path: no icon in FA.
  • polldaddy: no icon in FA.

#2 in reply to: ↑ 1 @melchoyce
4 years ago

Replying to sami.keijonen:

Here are Genericons used and why.

  • expand: no similar icon in FA.

See above suggestions.

  • next: no similar icon in FA but plenty of arrows to chooce.

See above suggestions.

  • previous: no similar icon in FA but plenty of arrows to chooce.

See above suggestions.

  • pinned: no similar icon in FA.

We can use http://fontawesome.io/icon/thumb-tack/. (Sorry @davidakennedy, I know you just tweaked the sizing for this icon! Didn't realize it was still a Genericon.)

  • path: no icon in FA.
  • polldaddy: no icon in FA.

Let's remove these. I don't know if either really makes sense in a social menu anyway — I don't think either have public profiles.

#3 @melchoyce
4 years ago

  • Summary changed from Twenty Seventeen: Replace Genericons arrows with Font Awesome arrows to Twenty Seventeen: Replace remaining Genericons with Font Awesome icons

#4 @sami.keijonen
4 years ago

Do you want me to create patch for this?

I'm not familiar how commiting to Core works so any tutorials would be helpful.

#5 @sami.keijonen
4 years ago

I have a patch coming up. At least I think so:)

@sami.keijonen
4 years ago

#6 follow-up: @sami.keijonen
4 years ago

Not sure if I did this right but there is a patch file for test.

I used arrow-left and arrow-right. They seem a little thick so we might need to try long arrows or carets.

This ticket was mentioned in Slack in #core by stevenkword. View the logs.


4 years ago

#8 in reply to: ↑ 6 @melchoyce
4 years ago

Replying to sami.keijonen:

Not sure if I did this right but there is a patch file for test.

Your patch is good, thanks. :)

I used arrow-left and arrow-right. They seem a little thick so we might need to try long arrows or carets.

I agree. Might also need to adjust the size of the icons, based on how they look. Would you mind trying out long arrows and carets and add some screenshots before uploading a patch?

The angles might also need some slight tweaking to vertically align them with the text — right now the down angle on menus should probably be top: -1px; instead of -2px.

Thumbtack looks perfect :)

Thanks for continuing to work on this!

#9 @sami.keijonen
4 years ago

Would you mind trying out long arrows and carets and add some screenshots before uploading a patch?

Sure. I'll test them both.

The angles might also need some slight tweaking to vertically align them with the text — right now the down angle on menus should probably be top: -1px; instead of -2px

Ok, I'll check that too.

#11 @melchoyce
4 years ago

Neither's super ideal. I'm going to make some custom arrows using Font Awesome as a base to use instead. Can get that done today. Thanks @sami.keijonen!

#12 @sami.keijonen
4 years ago

Sounds good!

Let me know where I can download arrow SVGs when you have time to create them.

It's bed time here in Finland but I can continue tomorrow.

#13 @melchoyce
4 years ago

Thanks! I'll post a link to download the new SVGs in this ticket. Have a good night :)

This ticket was mentioned in Slack in #core by helen. View the logs.


4 years ago

#15 @davidakennedy
4 years ago

  • Type changed from enhancement to defect (bug)

#16 @melchoyce
4 years ago

Let's try these SVGs: https://cloudup.com/cjUkmNNKlFM

Let me know if they're weird or broken. The size might also need to be adjusted to fit better.

#17 @sami.keijonen
4 years ago

Here is second patch with new arrow icons. I only used left and right arrows and adjusted the sizes a little bit. But I run out of time so they still might need some style details.

#18 @laurelfulford
4 years ago

@sami.keijonen, your patch is really solid! I made a couple tiny changes in 38488.2.patch based on reviewing with @melchoyce:

  • Moved the 'scroll down' arrow up a little bit.
  • Renamed the two new arrow icons for consistency - they originally had 'twentyseventeen' in the name, which was carried over from the Illustrator files.

#19 @sami.keijonen
4 years ago

Nice!

Let me know if you need more help.

This ticket was mentioned in Slack in #core-themes by sami.keijonen. View the logs.


4 years ago

@davidakennedy
4 years ago

Removes code from other patch.

#21 @davidakennedy
4 years ago

  • Keywords needs-refresh added

@laurelfulford Looks like your patch had some code in it from a patch for #38395. 38488.3.patch removes that. However, the fallback for IE 8 is not working. It needs a bit more work. :)

#22 @laurelfulford
4 years ago

Ah geeze - thanks @davidakennedy, and sorry about that! I'll look into the IE8 issue.

@laurelfulford
4 years ago

Adds fix for missing SVG fallback in IE8.

#23 @laurelfulford
4 years ago

Looks like a couple classes were not updated to match the new icon names in the SVG fallback styles in style.css. 38488.4.patch adds an update for that.

#24 @davidakennedy
4 years ago

  • Keywords has-patch added; needs-refresh removed

#25 @davidakennedy
4 years ago

  • Owner set to davidakennedy
  • Resolution set to fixed
  • Status changed from new to closed

In 39072:

Twenty Seventeen: Replace remaining Genericons with Font Awesome icons

  • Replaces: icon-pinned with icon-thumb-tack
  • Replaces: icon-next with icon-arrow-right
  • Replaces: icon-previous with icon-arrow-left
  • Replaces: icon-expand with icon-angle-down
  • Removes: Path, Polldaddy

Props sami.keijonen, melchoyce, laurelfulford.

Fixes #38488.

Note: See TracTickets for help on using tickets.