Page MenuHomePhabricator

Push typehead browse result selector button down one <div>

Authored by epriestley on Jun 21 2016, 10:12 PM.



Fixes T11190. The div with all the stuff in it was sometimes ending up on top of the "select" button, making it unclickable.

Test Plan

Clicked "select" in several browsers.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Push typehead browse result selector button down one <div>.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

What do you think about making the entire area clickable and have the [ Select ] be [ View ] instead?

I think it's really weird for text to be an action and a button to be a link. That's opposite from how I expect interfaces to work.

I mis-click the row all the time, since that's how typeaheads work. :(

I'm fine skipping the "View" part then, and just have the row work as well like we do on the ObjectSelector?

chad edited edge metadata.
This revision is now accepted and ready to land.Jun 21 2016, 10:22 PM

More specifically:

  • Making the entire thing a selector seems reasonable, although the element doesn't look very selectable to me. We can add hover, but I worry that will only be a weak hint. We could make the names links to hint that they're clickable.
  • Doing this may be a nontrivial amount of work, I think the dialog is currently cheating and taking advantage of the fact that Workflow dialogs have some special behavior when you click a <button />. Tractable, but not 5 seconds of work.
  • I think "View" on a button is weird, but just dropping a link there is also kind of weird. I don't particularly like the "[^]" icon thing we have going on in the main objet selector, either. Maybe an icon-link on the right or something? Although this may trip up existing users.

Oh, do you actually use this UI? I essentially never use it...

This might not be as bad as the "tags". I just always went to click on the "tag" when I saw the one I wanted since it looked like a tokenizer.

Just ship this and if I keep mis-clicking I'll look into a fix myself.

Yeah, that makes sense -- let's table this for now since I don't really want to rewrite Workflow yet and take another look after the main ObjectSelector gets updated? I'd like to fix that one too since I don't really like the current solution there, and maybe making them look and work more similarly is a good approach.

This revision was automatically updated to reflect the committed changes.