1478435 - Apply photon-styling to autocomplete popup

Status: VERIFIED FIXED

(DevTools :: Shared Components, enhancement, P1)

Comment 1

[:Harald Kirschner :digitarald]

ni? Victoria, we talked about this briefly in Slack, which guidelines would you propose?

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

There's https://design.firefox.com/photon/components/input-fields.html#dropdown-list in photon design system

Comment 3

[:Harald Kirschner :digitarald]

Attachment: Current look in Inspector

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: photon-autocomplete-popup.png

Here's with the patch styles applied in different situation (webconsole input, inspector markup search, rule view property autocomplete)

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1478435 - Apply photon-styling to autocomplete popup; .

This removes the border-radius from the autocomplete popup (and
its items) and refines the spacing.

Review commit: https://reviewboard.mozilla.org/r/259572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259572/

Comment 6

[Victoria Wang [:victoria]]

This is looking good! A few thoughts:

- Background color: Instead of the current gradient-looking backgrounds, we could match the new doorhanger background colors. It might make sense to match the foreground colors as well for a more contrasted/prominent look.

- Console autocomplete popup could be positioned 1px higher so that the input border is visible

- Left padding: This might be for a separate patch, but it I think it would be more polished to have a bit more left padding combined with positioning further to the left. For example, 4px more padding-left and -6px margin-left, or whatever styling to make the autocomplete text line up horizontally with the input text :)

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Attachment: popup-flat-background.png

This is with the colors (background, text, borders) of the doorhanger.
I find it a bit too subtle here, and I don't want to apply the doorhanger box-shadow which is very strong.
What do you think victoria ?

Comment 8

[Victoria Wang [:victoria]]

These are looking great to me :D, but I see what you mean about it looking a bit more subtle now in light theme. It seems more prominent in dark theme thanks to the lighter bg color.

Re: shadows: If I'm understanding correctly, it looks like all the Firefox doorhangers including the DevTools one are still randomly suffering from the double-shadow bug on OS X. If there's a way to use the OS X native shadow only, I think it would look a lot nicer. 

Here's a rough modification of your screenshot that shows ideas for improving the look:
- slightly darker shadows
- more left padding 
- positioned more to the left so that the text is lined up

https://mozilla.invisionapp.com/share/8SN95FUXK7C#/screens

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Attachment: popup-flat-bg-padding.png

Here we go with more inline-padding and offset to align the text from the popup items to the input

Comment 10

[:Harald Kirschner :digitarald]

Those changes look really good!

Comment 11

[Victoria Wang [:victoria]]

Yes, I think this looks very nice! Not too be too pixel-obsessed but I think it still needs to be positioned just 1 or 2 more px to the left to perfectly line up :)

Comment 12

[Tim Nguyen :ntim]

Comment on attachment 8995077 [details]
Bug 1478435 - Apply photon-styling to autocomplete popup; .

https://reviewboard.mozilla.org/r/259572/#review267762

::: devtools/client/themes/common.css:52
(Diff revision 1)
>  /* Autocomplete Popup */
>  
>  .devtools-autocomplete-popup {
>    box-shadow: 0 1px 0 hsla(209,29%,72%,.25) inset;
>    background-color: transparent;
> -  border-radius: 3px;
> +  border-radius: 0;

This is default, it should be possible to remove this.

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8995077 [details]
Bug 1478435 - Apply photon-styling to autocomplete popup; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259572/diff/1-2/

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Victoria Wang [:victoria] from comment #11)
> Yes, I think this looks very nice! Not too be too pixel-obsessed but I think
> it still needs to be positioned just 1 or 2 more px to the left to perfectly
> line up :)

yeah, I think it's good now, I needed to take the popup border into account.

Comment 15

[Victoria Wang [:victoria]]

Comment on attachment 8995077 [details]
Bug 1478435 - Apply photon-styling to autocomplete popup; .

Forgot to clear the review again! Excited to see this land.

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8995077 [details]
Bug 1478435 - Apply photon-styling to autocomplete popup; .

https://reviewboard.mozilla.org/r/259572/#review267984

auto r+ here to carry the one Victoria put on Bugzilla

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/419b4035f578
Apply photon-styling to autocomplete popup; r=nchevobbe.

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/419b4035f578

Comment 19

[Cristian Fogel [:cfogel]]

Verified with 63.0b11 on macos 10.9 design and colors used resemble the mockup