we have icons

What if we just removed the icons completely?

Honestly, half assed thought is currently:

I'm just going to steal them.

I don't think any icon is ever really going to say "Revision", "Audit", "Lint", "Build", "Task", "Commit", etc., no matter how many fancy illustrators we hire.

But yeah, all our most important icons are terrible. I'm a bad designer.

I plan to fix table and tweak the design of list more, I just needed to see it with some real data.

The problem is "gear" doesn't say "revisions" to me, without the check it's just a menu looking thing.

Without changing the design much:

We could maybe flip them. IDK...

Yeah, I think it's really hard to layer the icons and get top notch design.

Also: one of these commits has an important build failure, can you find it in less than 15 minutes as a user with poor color vision?

I think that got things closer but we aren't quite there yet. I'll poke at this a bit more tomorrow.

Possibly fallout from T11784 / D17911, although those shouldn't really interact here unless there are other bugs.

T11997/T12032 look related, but not the same behavior.

Forgot to specify browser: this is Chrome 58.0.3029.110 (64-bit) on OS X and Linux. Also replicates on Safari 10.1.1 (12603.2.4).

For pulls (vs mirrors), we switched to ls-remote to detect no-ops in D17497. The primary motivation was to let us make better clustering decisions, but it also "appeared" to improve performance, which suggests that it may not be outlandish to imagine that git ls-remote would use fewer resources here as well.

Thanks for the detailed response! We hadn't considered that people might tamper with mirror remotes, so you make a very good point there that our proposed change might cause a regression. And you're quite right that logging is our problem.

I'm not sure, since I'm not really sure what problem we're solving. It sounds like there are potentially three motivations:

I actually wasn't sure which to pick myself, since it's not as specific as most bug reports, but it seemed more like undesirable behavior of an existing feature, as opposed to a new feature. Would it be more appropriate to consider this as a feature request instead?

Can you walk me through why you consider this to be a bug report rather than a feature request?

Sounds good. Current trajectory is:

If you're making PHUIXButtonView I can come behind and merge in circle stuff. I don't think that needs to be a separate PHP class. I just need to dig out all the button-css stuff and make it more modular so it's not silly with overrides.

bad zoot

what is this even

(I don't have any plans for making JS and PHP render from the same authority -- I don't see any realistic way to accomplish it.)

Oh there is no PHUIXButtonView...

And, uh, SIMPLE is a color. I'm going to change that first, unless you have some other plan for having the yellow and non-yellow buttons present in the mock?

Though I do circle in JS in lightbox, so there would be a second application

You can just make the circle button a square button as well.

Is there a place in 2-5 years where we can write one UI library and work in PHP and JS

Yes I slimmed them in photoshop :(

Thanks! That should be perfect.

Okay. I have to port whatever we're using to PHUIX so you'll have to retouch both PHP and JS, but I can maybe put PHUIX rendering beside PHUI rendering in UIExamples to make that easier.

Just use SIMPLE for now and I'll retouch as needed. It's probably a little over styled in the mock.

Are those buttons rendered by any existing class, or are they novel elements? The circular menus are presumably PHUIIconCircleView but the other buttons look custom (it looks like the spacing is slightly different from the existing "Simple" buttons, maybe)? Should I add a new mode to PHUIButtonView, or is this an existing mode somewhere that I'm just missing, or are they just "Simple" buttons with slightly Photoshoppy rendering?

If you believe this is not an issue with a bad SAPI binary, please file a new bug report including reproduction steps I can follow to reproduce the issue (see Providing Reproduction Steps for guidance).

Possibly also, add a "See All" link to ApplicationSearch

Sounds reasonable to just improve that.

Yeah, I tried hard to not make it progress-like. My approach was mostly, "if I have a dozen comments across 5 files, how could I assume I've read/addresses each piece of feedback" - and that still leads to some sort of progress bar. If people don't click "done" I think it's reasonable to assume they don't need that level of organization. I also suffer from ADHD and have a difficult time if I don't maintain things in a list.

I'm fine with dropping "no progress bars". I believe most requests we've ever received about inlines -- like T8909#224084 -- are fundamentally misguided. That user wanted features to make managing a review with 100 spelling corrections easier. But these requests are common and I don't have any real reason to believe that making Differential hard to misuse is actually helping anyone or putting any real pressure on attacking the root issues.

I think this feature is just inherently incompatible with my hardline "review shouldn't be a progress bar" view, and it would be more confusing to try to obscure the progress aspect of it.

Hey, we're also interested in this by way of T12545 (which was merged in above). A couple of our teams have historically assigned their oncall/tech-debt tasks to a mailing list user, and used the priority-sorted view of those tasks to run their oncall shifts. That workflow no longer works with the reverse-chronological sort.

What about hiding the x/n count until you've marked 1 as done?

I can also just put next/prev back on inlines if you prefer, and we can deal with the design issue if/when we add a context menu for "View History", "View Raw Remarkup", etc. (T11401).

I don't think we need to keep iterating here -- I don't think any version of this will ever be useful to me personally, and I think I'm clearly fighting a losing battle by trying to keep all the "X / Y" progress-bar-style elements out of the UI.

I also have some ideas around a more minimal UI if you'd like to see one more pass.

hmmm, I'm happy to take another pass here. For myself I feel like you, I don't need it - if I keep reviews small - but I think that's not something we can apply across all our users. I've also found I don't need "done" until I do. There reaches a point where I want to make sure I've addressed everything before sending it back for review, and then that list I feel is really useful.

I think it's fine for none of this to work on mobile for now.

(I'm also ok with none of this on mobile)

Do you want a mobile design? I think we can just hide a div on desktop?

Okay, I'm going to implement these behaviors:

Cool, Safari seems fixed again.

(◠‿◠✿)

Oh! Works great in Chrome.

Have you tried Chrome?

I deployed D18042 to secure, but it isn't retroactive. You can either:

Specifically, I got this (mail with HTML and text parts containing over-escaped HTML):

On top of this, I got some email from Pholio with odd HTML behavior, but I can't immediately reproduce it and I don't have any guesses about what's causing it so I'll assume I'm just crazy until someone else hits it.

(See also M1476.)

I've got most of a diff for this.

It's still probably Safari though

Probably permissions, let me see if I can reproduce locally. I thought I tested that but maybe didn't get the "replace an image" transaction specifically.

I tried logged out Safari and saw no images

oh, maybe file permissions then

What I'm seeing, in Chrome?

Safari only

What I'm seeing:

The first two images seem to have been lost, Pholio bug?

Next steps:

This doesn't appear to describe a bug in Phabricator.

I'm just going to nuke this since there's no repro. This workflow has also been rewritten on the experimental branch and the specific error encountered here should now be impossible.

You can leave comments on the left hand side of a 2-up view -- that is, scrolling in EITHER of these areas on 2-up would trigger the problem: