Page MenuHomePhabricator

Move Timeline icons to Fonts
ClosedPublic

Authored by chad on Apr 20 2014, 9:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 11:02 AM
Unknown Object (File)
Wed, Dec 11, 3:02 AM
Unknown Object (File)
Sat, Nov 30, 1:39 AM
Unknown Object (File)
Sat, Nov 30, 1:38 AM
Unknown Object (File)
Sat, Nov 30, 1:38 AM
Unknown Object (File)
Sat, Nov 30, 1:38 AM
Unknown Object (File)
Sat, Nov 30, 1:34 AM
Unknown Object (File)
Sat, Nov 30, 1:26 AM
Subscribers
Tokens
"Grey Medal" token, awarded by btrahan.

Details

Reviewers
epriestley
btrahan
Commits
Restricted Diffusion Commit
rP11fd6afeb10e: Move Timeline icons to Fonts
Summary

Throwing this up for testing, swapped out all icons in timeline for their font equivelants. Used better icons where I could as well. We should feel free to use more / be fun with the icons when possible since there is no penalty anymore.

Test Plan

I browsed many, not all, timelines in my sandbox and in IE8. Some of these were just swagged, but I'm expecting we'll do more SB testing before landing.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

chad retitled this revision from to Move Timeline icons to Fonts.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added reviewers: epriestley, btrahan.
src/view/phui/PHUIIconView.php
87

is there a better way to do this? want basic dead simple usage.

src/view/phui/PHUIIconView.php
87

This seems fine to me, although strstr() will match fa-doughyperson because it contains gh. Although it doesn't matter here, strtstr() also has unnecessarily bad performance if the haystack is large and the needle matches early, because it must copy most of the haystack to return it. This is probably better, since it won't match ghost or fa-doughyperson, will never examine more than 3 characters, and runtime is independent of the size of the string.

if (!strncmp($this->iconFont, 'gh-', 3)) { ... }
  • Test out full-on FontAwesome
epriestley edited edge metadata.

Looks good to me technically.

This revision is now accepted and ready to land.Apr 22 2014, 3:13 PM

Low risk if we break anything. Will land and see if we broke anything before replacing all the action icons.

chad updated this revision to Diff 20970.

Closed by commit rP11fd6afeb10e (authored by @chad).