Page MenuHomePhabricator

Make closed objects in global typeahead look closed
ClosedPublic

Authored by amckinley on May 25 2017, 12:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 10:01 PM
Unknown Object (File)
Thu, Dec 19, 7:41 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Unknown Object (File)
Thu, Dec 19, 2:43 PM
Subscribers

Details

Summary

Fixes T6906.

I found the code in behavior-search-typeahead.js that was throwing away the closedness-detction work done in Prejab.js::transformDatasourceResults. Modified it to re-add the correct class name to the phabricator-main-search-typeahead-result elements.

Then I found some CSS in typeahead-browse.css and completely flailed around until realizing that particular CSS only gets loaded when hitting the typeahead endpoint directly. Copied the relevant bit of CSS over to main-menu-view.css (but maybe it should be removed from typeahead-browse.css?).

This is my first JS/CSS change, so please don't assume I did anything right.

Test Plan

Screen Shot 2017-05-24 at 5.20.42 PM.png (182×433 px, 16 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

.phabricator-main-search-typeahead-result probably shouldn't be split into two different CSS files, that's a little confusing. If the CSS is only for the search typeahead, it can stay in main-menu-css. I'd probably move the other rules over as well and remove the .typeahead-browse-item. I'd maybe look at blame to see how it ended up scoped there just in case I'm missing something.

Also, I've been using this rule for making the image look disabled as well.

opacity: .8;
-webkit-filter: grayscale(100%);
filter: grayscale(100%);
  • move remaining phabricator-main-search-typeahead-result directives to main-menu-view.css
webroot/rsrc/css/aphront/typeahead-browse.css
68

This rule was already defined with different settings in main-menu-view.css, so I just removed this version of it.

Do you mind greying out the image too with this CSS too?

In D18017#218480, @chad wrote:

Also, I've been using this rule for making the image look disabled as well.

opacity: .8;
-webkit-filter: grayscale(100%);
filter: grayscale(100%);
This revision is now accepted and ready to land.May 25 2017, 8:33 PM

Sure, I'm just trying to figure out how to apply a style to an image that's actually a background-image set on a <span> element.

This is how the image is getting created currently:

var attr = {
  className: 'phabricator-main-search-typeahead-result'
};

if (object.imageURI) {
  attr.style = {backgroundImage: 'url('+object.imageURI+')'};
}

Probably right on .phabricator-main-search-typeahead-result.result-closed. I think it's fine to just hit the whole div in that case.

  • apply grey filter to closed images
This revision was automatically updated to reflect the committed changes.