Page MenuHomePhabricator

Don't communicate action on older diffs exclusively through the use of color/tooltips
AbandonedPublic

Authored by epriestley on Apr 30 2016, 12:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 24, 10:47 PM
Unknown Object (File)
Wed, Apr 24, 10:47 PM
Unknown Object (File)
Wed, Apr 24, 3:39 AM
Unknown Object (File)
Thu, Apr 11, 7:41 AM
Unknown Object (File)
Wed, Apr 3, 7:01 AM
Unknown Object (File)
Wed, Mar 27, 9:37 PM
Unknown Object (File)
Mar 24 2024, 2:56 AM
Unknown Object (File)
Mar 21 2024, 6:56 PM
Subscribers

Details

Summary

Fixes T10906. Not sure if you have a better fix for this, but just drop the background on the "Accepted/Rejected Older" variants of these icons.

Test Plan

Faked these statuses, looked at the icons.

Diff Detail

Repository
rP Phabricator
Branch
coloricon
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12001
Build 15098: Run Core Tests
Build 15097: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Don't communicate action on older diffs exclusively through the use of color/tooltips.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
  • We have a similar UI element in Audit but it's not possible to update commits so there are no "Older" statuses there.
  • I didn't bother trying to add these to PHUIStatusItemView::... constants because we ended up using this element in 3,000 places with hundreds of icons, and should probably just toss the ICON_... constants at some point.

I would probably keep the icons but just use the 'disabled-grey' look. My concern is new icons would introduce some added confusion.

That still only differentiates them through color though, right -- we'd just get a lighter grey? Or am I thinking of the wrong thing?

If we're going to fix this I think we should find a solution that uses something other than color to communicate the information, since even if one user can tell the difference between red and light grey better than red and dark grey, other users won't be able to.

Colorblind doesn't mean you can't see contrast. I think it's still more useful to have one icon, two visual states, than learn new icons. The difference between and is too nuanced to understand previous vs. current. If it's helpful, we could just also add the text for "previous diff" as a note in PHUIStatus ?

So the proposal would be:

btrahan
epriestley (previous diff)
chad (previous diff)

epriestley edited edge metadata.

More aggressive version, not really happy with this.

Screen Shot 2016-04-29 at 6.08.49 PM.png (187×435 px, 23 KB)

I think there's some kind of bug with the highlighting interacting with the notes field.

The highlighting also communicates information only through the use of color so we should probably fix that, too.

I'm going to shelve this for tonight and cut the release, I'll play with it some more at some point.

Theres a task filed on the highlight, I just haven't fixed it yet. Pretty sure it's two-column stuff.

let me dig through font-awesome see if maybe there are better icons to pick here.