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.
Details
- Reviewers
epriestley btrahan - Commits
- Restricted Diffusion Commit
rP11fd6afeb10e: Move Timeline icons to Fonts
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
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)) { ... } |
Low risk if we break anything. Will land and see if we broke anything before replacing all the action icons.