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.
Details
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
- 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 ?
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.