1040923 - Implement updated styling to Awesome Bar

Status: RESOLVED DUPLICATE of bug 1181078

(Firefox :: Address Bar, defect)

Description

[Sevaan Franks [:sevaan]]

This bug is for the implementation of the visual improvements to the Awesome Bar handled in Bug 1034542.

Mockups:http://people.mozilla.org/~sfranks/AwesomeBar/Visual%20Improvements%20to%20Awesome%20Bar.pdf

Comment 1

[Tomasz Kołodziejski [:tomasz]]

I wanted to work on this bug. In the pdf I really like the changes slide where you explicitly state what should be changed. However, I have a couple of questions:

1. Should the changes be consistent among all the operating systems?
2. I've noticed the sample windows styling (after) has the bookmark star not shrunk. Was that on purpose?
3. What should happen if first/second line is too long to display? Keep in mind what alexbardas says about the ellipsis bug 1040725 comment 11.
4. Any specification for last visited? It looks like the algorithm you proposed is like this:
   a) if today then "today at time"
   b) if yesterday then "yesterday"
   c) if <week then "count days ago"
   d) if <month then "count weeks ago"
   e) if <year then "count months ago"
   f) else "count years ago"
   Do you want any more for today? Like "just now" or "10 minutes ago"? I think we don't have such functionality right now so we need to specify it.

Comment 2

[Tomasz Kołodziejski [:tomasz]]

In point 4 above I just realized that yesterday/today are a little bit different than "x ago" (it's not relative). So I'm not sure we want to treat them as proposed. If you searched sth 20 minutes ago and it's 10 minutes past midnight you'll get "yesterday" which will be so much different from searches you are doing now "today at 0:10". It's also timezone dependent which is, however, not such a big deal.

Comment 3

[Tomasz Kołodziejski [:tomasz]]

Attachment: Screen Shot 2014-08-26 at 4.30.35 PM.png

We also unnecessarily reserve the space for bookmark star icon.

Comment 4

[Tomasz Kołodziejski [:tomasz]]

I'm seeing some code in autocomplete.xml (look for "type == "keyword"") that suggests that we're showing something some visual feedback for keywords. I tried a couple of times but I don't see this code in action at all. I think it's a good bug to address this as well.

It's for me a little confusing that in the code it looks like tag and keyword are mutually exclusive but in the UI they are not.

Comment 5

[Tomasz Kołodziejski [:tomasz]]

For the keyword case: we're showing magnifying glass for the bookmark with keyword only if there is a %s in the url, which actually makes sense.

Comment 6

[u428464]

FWIW the visual spec is here : http://people.mozilla.org/~mmaslaney/Search/AwesomeBar-Spec.png

Comment 7

[Sevaan Franks [:sevaan]]

Hi Tomasz,

That's great you want to take this on!

My styling is a little out-dated. In Bug 1041736 :mmaslaney gave the design some visual polish, and he includes information on Windows styling, so please look at that stuff, and not what's in my PDF.

http://people.mozilla.org/~mmaslaney/Search/AwesomeBar-Spec.png

So that should answer questions 1 & 2.

Regarding 3, I think it should trail off into ellipses. I'm not sure I understand comment 11 in that bug you linked to. Can you resummarize?

4: How do you feel about keeping the Today at X:XXam/pm, but if the site you visited was within the last hour, then it says X minutes ago?...so up to 59 min ago, and then it switches to the Today (or Yesterday) at Time.

Comment 8

[Tomasz Kołodziejski [:tomasz]]

To be honest I didn't expect so many changes of behavior in this bug. With the updated design it looks like there are a lot and judging from bug 951624 there's not even a slightest chance we can do it in one bug.

So my guess is that we can here do the points listed in the pdf but to comply with the png spec, that is just do the visual changes plus introduce "last visited" text.

Otherwise, we have a ton of questions that need to be asked and the bug should be split into small ones.
1. Nit: what about transparency/rounded corners as in pdf?
*. It looks like the behavior on the image is completely different from what we see today. To summarize:
2. By default we want to show in the location bar: magnifying glass, search engine name (please keep in mind that "(search engine name) Search for:" may not always make sense. I think "Wikipedia Search for:" pretty much does not). Is that correct?
3. What is "Hide search suggestions"? Is it exactly the same button as in "Manage search engines -> Show Search Suggestions"? 
4. What happens to the location bar when we focus some history result?
5. How do we comply with what's being done in bug 951624?
5. As far as I know there are no search suggestions in the Awesome Bar right now. Is it discussed anywhere? For example: how do we fit the suggestions in one line? (see suggestions for "what is the name") How am I supposed to choose the suggestion I want? Tab into that line and then use right/left arrow? Sounds very inefficient.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

I spoke with Tomasz, and clarified that this bug is only about the styling, and not any of the added functionality (suggestions, first result item, "last visited").

Comment 10

[Tomasz Kołodziejski [:tomasz]]

Attachment: flexbox-awesomebar.html

Proposed behavior for the first line text overflow. There's a favicon on the left, title text in the middle (which is being cropped if there is not enough space) and optional star on the right.

It basically only moves the star to the left but and does the cropping using css-only, yay! Also I don't reserve any space for the star like in my previous screenshot.

Can you please have a look if it's the behavior we want?

Comment 11

[Sevaan Franks [:sevaan]]

Confirmed! The demo worked great.

Comment 12

[Tomasz Kołodziejski [:tomasz]]

Attachment: updated-awesomebar-simpler.patch

I'm sorry for not asking on IRC before actually bothering you with feedback? :paolo. It looks like you know a lot about this code. Can you have a look?

I've been playing with moving this code to html because it has free overflow support. However, I'm not entirely sure if that's the optimal solution.

So moving to html/flexbox gets rid of the js ellipsis (see the demo previous attachment 8480906 [details]). But on the other hand we have to explicitly know the width of the popup and set it using js because otherwise the outer xul box container will increase its length enough to fit the whole text and the ellipsis won't show up. This is problematic because I don't know how to detect the resize of that panel (it would be a bad idea to listen to window resize).

I'll be happy to see your feedback and advice whether this makes any sense or should I stick to the ellipsis hack.

Comment 13

[Tomasz Kołodziejski [:tomasz]]

Attachment: updated-awesomebar-xul.patch

It isn't working yet but I'm updating the patch with my today's work. The first row is pretty much working: try having too long title or too long search term. I'm very satisfied with the overflow working with css ellipsis. The only hack here is to set fixed width on the outer html container (_setupWidth in the code).

I expect that the whole mock can be implemented with this simple concept and some  enabling/disabling of boxes (_urlBox, _actionBox in the code. I'm going to add _tagsBox). Flexbox behaves very well when I just hide some of its children.

I hope it's ok for me to use html and its mighty flexbox.

Comment 14

[:Paolo Amadini]

Comment on attachment 8485323 [details] [diff] [review]
updated-awesomebar-xul.patch

(In reply to Tomasz Kołodziejski [:tomasz] from comment #12)
> :paolo. It looks like you know a lot about this code. Can you have a look?

Hi Tomasz, sorry for the late response - the work week kept me quite busy! I wasn't involved in writing this code, but I'll be glad to provide feedback and help answer any questions around it when I can.

> I've been playing with moving this code to html because it has free overflow
> support. However, I'm not entirely sure if that's the optimal solution.

I guess the reason for manual overflow handling wasn't the lack of support in XUL (if I remember correctly, XUL labels can be configured to handle overflow automatically), but to show or hide the tooltip displayed when hovering the cursor over the truncated text. This would mean that changing these elements to use HTML wouldn't provide much value by itself.

> But on the other hand we have to explicitly
> know the width of the popup and set it using js because otherwise the outer
> xul box container will increase its length enough to fit the whole text and
> the ellipsis won't show up.

This, and the fact that we may have to debug and ensure the accessibility, keyboard navigation, and right-to-left localization properties of the new HTML elements don't regress compared to the current XUL behavior, make me think that we should stick to the existing approach.

Comment 15

[Tomasz Kołodziejski [:tomasz]]

I've heard that the reason is that when this code was written there was no CSS ellipsis. What's more, support for XUL is really weak. It only works for labels as you said but we have "html labels" that is "this is <span>highlighted</span> text". XUL does not support that.

I talked to :adw and it looks like accessibility is the biggest deal. right-to-left support is built into flexbox so that would work (I'd be more concerned with current ellipsis in rtl but since it's there for a long time it surely works). I'll try to implement these changes with minimal impact using XUL.

Comment 16

[Tomasz Kołodziejski [:tomasz]]

Hey Sevaan, I need updated icons: tag (chrome://browser/skin/places/tag.png) and switch-to-tab  (chrome://browser/skin/actionicon-tab.png) and possibly favicon (chrome://browser/skin/places/star-icons.png). I need them in both normal and high-dpi.

Also, the png above is missing the keyword search look. If you want to change it (or change the magnifying glass icon) then I'd need that as well.

Comment 17

[Sevaan Franks [:sevaan]]

Attachment: Updated Awesome Bar Spec

Attaching the updated Awesome Bar visual spec by mmaslaney.

Comment 18

[Sevaan Franks [:sevaan]]

(In reply to Tomasz Kołodziejski [:tomasz] from comment #16)
> Hey Sevaan, I need updated icons: tag (chrome://browser/skin/places/tag.png)
> and switch-to-tab  (chrome://browser/skin/actionicon-tab.png) and possibly
> favicon (chrome://browser/skin/places/star-icons.png). I need them in both
> normal and high-dpi.
> 
> Also, the png above is missing the keyword search look. If you want to
> change it (or change the magnifying glass icon) then I'd need that as well.

Mike, do you have these assets?

Comment 19

[Michael Maslaney [:mmaslaney] ([e-mail])]

Tomasz, would you like the assets in png or svg format?

Comment 20

[Tomasz Kołodziejski [:tomasz]]

Svg is fine.

Comment 21

[Tomasz Kołodziejski [:tomasz]]

Attachment: updated-awesomebar-xul.patch

I've been fighting with the XUL. I couldn't accomplish the design as in my HTML mockup anyhow and so I ended up using flexbox nonetheless but this time with XUL. My biggest problem was to have star right after the title text. That's because the js ellipsis works only with flex="1" box which it exactly what I didn't want.

Please have a look whether the XUL changes makes sense. I have a question: why it didn't work without the additional <xul:hbox>? (you will notice one hbox that I added just because it worked with it)

Don't pay attention for the color changes in this patch. That part is not ready yet. But the boxes should be in (more or less, not pixel-perfect yet) right places.

Comment 22

[:Paolo Amadini]

Comment on attachment 8486128 [details] [diff] [review]
updated-awesomebar-xul.patch

Hi Tomasz, I haven't really worked with XUL in the past few months, I could definitely review this but it would still take quite some time.

Jared, maybe you can answer Tomasz questions on the XUL code better than me?

Comment 23

[Tomasz Kołodziejski [:tomasz]]

Ping dao. I wanted to get back to this bug but I still need some feedback on XUL. What's even worse, as far as I know this got a little outdated now.

I also need some help with colors. I discussed it with Alex and it looks like changing colors is not that simple due to themes support (and also high contrast stuff in some old Windows).

If you can't give feedback about that dao, please let me know.

Comment 24

[Dão Gottwald [:dao]]

Comment on attachment 8486128 [details] [diff] [review]
updated-awesomebar-xul.patch

>   <binding id="autocomplete-richlistitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
>     <content>
>-      <xul:hbox align="center" class="ac-title-box">
>-        <xul:image xbl:inherits="src=image" class="ac-site-icon"/>
>-        <xul:hbox anonid="title-box" class="ac-title" flex="1"
>-                  onunderflow="_doUnderflow('_title');"
>-                  onoverflow="_doOverflow('_title');">
>-          <xul:description anonid="title" class="ac-normal-text ac-comment" xbl:inherits="selected"/>
>+      <xul:hbox align="center" class="ac-title-box" style="display:flex">
>+        <xul:image xbl:inherits="src=image" class="ac-site-icon" style="flex-shrink:0"/>
>+        <xul:hbox>
>+          <xul:hbox anonid="title-box" class="ac-title"
>+                    onunderflow="_doUnderflow('_title');"
>+                    onoverflow="_doOverflow('_title');">

Looks like you're mixing flexbox and XUL layout here, which seems extraordinarily brittle, because there's no spec defining how these things should interact. I would strongly advice against doing this and I can't really answer questions about why this layout mix behaves in certain ways that you didn't intend.

I would also recommend morphing this to a meta bug tracking smaller changes in separate bugs. I don't think it's necessary nor efficient to implement the entire design update in one monolithic patch.

Comment 25

[Tomasz Kołodziejski [:tomasz]]

I'm still waiting for the svg icons mmaslaney. Can you post them here?

Comment 26

[u428464]

AFAIK the assets are attached in bug 1041736.

Comment 27

[Michael Maslaney [:mmaslaney] ([e-mail])]

Attachment: Attached, the AwesomeBar svg assets.

Comment 29

[Drew Willcoxon :adw]

Getting this off of Tom's plate.

Comment 30

[Marco Bonardo [:mak]]

This has been superseded by bug 1181078, so I'm closing it. In case we can still recover resources from here.