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)
Sat, Nov 9, 1:52 PM
Unknown Object (File)
Fri, Oct 25, 2:36 AM
Unknown Object (File)
Oct 21 2024, 2:47 PM
Unknown Object (File)
Oct 16 2024, 5:51 PM
Unknown Object (File)
Oct 5 2024, 12:53 AM
Unknown Object (File)
Sep 21 2024, 12:18 AM
Unknown Object (File)
Sep 12 2024, 9:03 AM
Unknown Object (File)
Sep 12 2024, 9:03 AM
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
Branch
T6906 (branched from master)
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17248
Build 23097: Run Core Tests
Build 23096: arc lint + arc unit

Unit TestsFailed

TimeTest
249 msPhabricatorCelerityTestCase::Unknown Unit Message ("")
Assertion failed, expected 'true' (at PhabricatorCelerityTestCase.php:32): When this test fails, it means the Celerity resource map is out of date. Run `bin/celerity map` to rebuild it. ACTUAL VALUE
1 msAlmanacNamesTestCase::Unknown Unit Message ("")
30 assertions passed.
0 msAlmanacServiceTypeTestCase::Unknown Unit Message ("")
1 assertion passed.
0 msAphrontHTTPSinkTestCase::Unknown Unit Message ("")
4 assertions passed.
0 msAphrontHTTPSinkTestCase::Unknown Unit Message ("")
6 assertions passed.
View Full Test Results (1 Failed · 337 Passed)

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.