Closed Bug 988481 Opened 10 years ago Closed 10 years ago

Implement Windows styling of Translation Infobar

Categories

(Firefox :: Theme, defect)

x86_64
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: MarcoM, Assigned: mak)

References

Details

(Whiteboard: [translation] p=5 s=it-32c-31a-30b.2 [qa-])

Attachments

(2 files, 6 obsolete files)

Implement Windows styling of translation info bar.
Depends on: 988478
Winows ? Typo ?
Summary: Implement Winows styling of Translation Infobar → Implement Windows styling of Translation Infobar
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: [translation] p=0 → [translation] p=5
It'd be nice if all 3 designs were consistent. It'd also be nice to use this styling globally on all infobars (sync, FHR, telemetry, ...)
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Whiteboard: [translation] p=5 → [translation] p=5 s=it-32c-31a-30b.1 [qa?]
Component: General → Theme
Whiteboard: [translation] p=5 s=it-32c-31a-30b.1 [qa?] → [translation] p=5 s=it-32c-31a-30b.1 [qa-]
So, I have implemented the mock-up, as defined here (https://bug988478.bugzilla.mozilla.org/attachment.cgi?id=8400844) , but I have some questions regarding it:

1. This seems to use in-content prefs styling, for example the font is Clear Sans, menulists and buttons are rounded, the light blue gradient looks the same. I would just like to have a confirmation this is what we should do here, included the font change.
2. I assume then the popups opened from the menulist and menu should be styled as the in-content prefs ones, and so also the menuactive entry should have the same light blue gradient as the Yes button?
3. The options button type="menu" at the end has a common dropdown glyph (down arrow), instead in-content prefs unify menulists and type="menu" buttons dropdown glyphs to a double arrow one. Should I unify them here or keep them different?
4. The dropdown graphic used by in-content prefs differs from the one used in the mock-up (http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/incontentprefs/dropdown.png) the arrows are more distant and it's bigger. For now I'm just using a transform: scale(.8) but that's not very nice, I may need a different graphic for this one, or to replace the existing one of in-content prefs?
5. The gradient hex values on the Yes button look wrong, I get a violet button instead of a light blue one. For now I used the in-content preferences ones #4cb1ff, #1792e5 with a box-shadow: inset 0 0 2px 0 rgba(255,255,255,0.2); what should I do?
6. I don't know how to handle hover state on the buttons and menulists, the in-content preferences style is so subtle that it's like not having it at all. For now I'm increasing the box shadow opacity, but I may well do nothing.
Flags: needinfo?(sfranks)
7. the border around the notification box (#bbc7d8) is completely different color than any other notification box, is that really wanted just for this one? I think I will actually leave this for bug 990633 since that's "external" styling and related to the arrow itself.
(In reply to Marco Bonardo [:mak] from comment #3)
> 1. This seems to use in-content prefs styling, for example the font is Clear
> Sans, menulists and buttons are rounded, the light blue gradient looks the
> same. I would just like to have a confirmation this is what we should do
> here, included the font change.

This seems backwards since we explicitly don't want this to be considered part of the content area, hence the arrow pointing to the URL bar, right?
(In reply to Dão Gottwald [:dao] from comment #5)
> This seems backwards since we explicitly don't want this to be considered
> part of the content area, hence the arrow pointing to the URL bar, right?

I have no idea, that's why I'm asking confirmation. Also, note that the arrow pointing to the urlbar won't appear if there's a toolbar (like the bookmarks toolbar) in the middle.
Attached patch patch v1 (obsolete) — Splinter Review
This is the current patch implementing the mock-up as-is
feedback-ing mano that takes care of the Mac theming.

The questions are clearly still open, in the meanwhile I tried to keep this as much isolated as possible and I tried to remove any non-immediately useful rule.
Attachment #8418813 - Flags: feedback?(mano)
Hi Marco,

Sorry for the delay in getting back with you. I had to meet with some of the other designers to answer some of these questions.

Background: Some work has been done on creating a unified style guide that will serve to answer these questions in the future. But that could be a couple weeks to a couple months away, so in the meantime let's try and stick close to the mockups.

Regarding your questions:


1) No, it is not tied to in-content prefs, even though some elements are similar. The font is based on the system, so please use the system font, and keep the button rounded, as that is what Windows currently does. Take cues from the buttons you see at the bottom when you are customizing in customize mode (Restore Defaults, Title Bar Button)

2) Use the system standard as well. This is more chrome than in-conent.

3) Stick with the single arrow, please!

4) Do we have a system glyph being used already? If we have it in the chrome UI, let's stick with that. Otherwise, we can add to the glyph set.

5 & 6) Decision: No Gradient... HEX: #0095DD; HOVER: #008ACB; ACTIVE: #006B9D;

7) We can leave it for that bug and I will preemptively come to a decision with the other designers.
Flags: needinfo?(sfranks)
(In reply to Sevaan Franks from comment #8)
> and keep the button rounded, as that is what Windows currently does.

Not Windows 8...

> Take cues
> from the buttons you see at the bottom when you are customizing in customize
> mode (Restore Defaults, Title Bar Button)

Indeed those buttons are rounded and don't look like Windows 8 native buttons, I can clearly stick to the mock-up/customize style, but...

> 2) Use the system standard as well. This is more chrome than in-conent.

...on Windows 8 the controls will be rounded but the popups squared... I can do that, but it may look a little bit "exotic".
Comment on attachment 8418813 [details] [diff] [review]
patch v1

applying feedback...
Attachment #8418813 - Attachment is obsolete: true
Attachment #8418813 - Flags: feedback?(mano)
Attached patch patch v2 (obsolete) — Splinter Review
This also includes the Linux styling for bug 988482.

I think it's a good starting point, there may be some details in need of tweaking but as I said I kept this very much self-contained, so polishing on top should be easy.
Attachment #8419292 - Flags: review?(mano)
Comment on attachment 8419292 [details] [diff] [review]
patch v2

>--- a/browser/themes/windows/browser.css
>+++ b/browser/themes/windows/browser.css

>+/* Translation infobar */
>+
>+%include ../shared/translation/infobar.inc.css
>+
>+notification[value="translation"],
>+notification[value="translation"] menulist {
>+  color: #545454;
>+  background-color: #EEE;
>+}
>+
>+notification[value="translation"] button,
>+notification[value="translation"] menulist {
>+  color: #545454;
>+}
>+
>+notification[value="translation"] button[anonid="translate"] {
>+  color: #FFF;
>+  background-color: #0095DD;
>+  box-shadow: none;
>+  border: 1px solid #006B9D;
>+}
>+
>+notification[value="translation"] button[anonid="translate"]:hover {
>+  background-color: #008ACB;
>+}
>+
>+notification[value="translation"] button[anonid="translate"]:active {
>+  background-color: #006B9D;
>+}

Why are you hardcoding these colors? How does this look when using a high-contrast theme?
(In reply to Dão Gottwald [:dao] from comment #12)
> Why are you hardcoding these colors? How does this look when using a
> high-contrast theme?

The short answer is because hardcoded colors are part of the requested design. Sounds like we may shortly have a new unified design reference, but that's not ready so I can't really re-use existing colors.
The request is to make it look the same, unregarded the Windows version or the linux wm, so no native colors.

Regarding high contrast, I don't know if the requirement differs there, I interpreted it as "it will look the same regardless of the OS theme", even if I know this is not usual (the above is not usual as well). If we want to keep system colors for hi-contrast theme, it should be feasible through a media query, but in all of the other cases I'm not sure ow I should go without hardcoding colors, for now.
(In reply to Marco Bonardo [:mak] from comment #13)
> The short answer is because hardcoded colors are part of the requested
> design. Sounds like we may shortly have a new unified design reference, but
> that's not ready so I can't really re-use existing colors.
> The request is to make it look the same, unregarded the Windows version or
> the linux wm, so no native colors.
> 
> Regarding high contrast, I don't know if the requirement differs there,

It generally does. If the design doesn't meat accessibility standards, the design is broken.

> I
> interpreted it as "it will look the same regardless of the OS theme", even
> if I know this is not usual (the above is not usual as well). If we want to
> keep system colors for hi-contrast theme, it should be feasible through a
> media query, but in all of the other cases I'm not sure ow I should go
> without hardcoding colors, for now.

When hardcoding stuff, our standard way is to put that behind -moz-windows-default-theme and fall back to native otherwise. Of course, there's nothing like -moz-windows-default-theme on Linux...
(In reply to Dão Gottwald [:dao] from comment #14)
> When hardcoding stuff, our standard way is to put that behind
> -moz-windows-default-theme and fall back to native otherwise. Of course,
> there's nothing like -moz-windows-default-theme on Linux...

Right, I'm going to fallback to native colors on Win, I think I'll leave Linux alone for now (and I guess Mac will end up being the same), since looks like we don't have a standard on how to handle that.
I guess this fact may become a recurring problem if we start doing more "unified" theming, but this is probably something that should be brought up with UX for the unified UI style proposal. I will file a UX bug follow-up that may use this translation experiment as an example to figure which kind of solution we want.
Attached patch patch v3 (obsolete) — Splinter Review
with -moz-windows-default-theme, tested with high-contrast themes on Win8.1.1
Attachment #8419292 - Attachment is obsolete: true
Attachment #8419292 - Flags: review?(mano)
Attachment #8419641 - Flags: review?(mano)
(In reply to Marco Bonardo [:mak] from comment #16)
> Created attachment 8419641 [details] [diff] [review]
> patch v3
> 
> with -moz-windows-default-theme, tested with high-contrast themes on Win8.1.1

Marco, I'm having a hard time setting up my machine to build Firefox. Do you mind posting screenshots as well?
sure, let me do some collage work.
(In reply to Marco Bonardo [:mak] from comment #18)
> sure, let me do some collage work.

Thanks a bunch!
Attached image win+hi-contrast+lin screenshot (obsolete) —
Attachment #8419692 - Flags: feedback?(sfranks)
Blocks: 988482
(In reply to Marco Bonardo [:mak] from comment #20)
> Created attachment 8419692 [details]
> win+hi-contrast+lin screenshot

Thanks Marco, this is making it much easier. There are a few things I can see to improve here, but overall it's looking great! Good job so far.

1) Increase the height of the infobar to 40px
2) The listmenu background should be white, not the same colour as the infobar.
3) Buttons should have a height of 30px
4) Try replace the Translate button CSS with this (hope I'm doing this right, forgive me if I'm not):

notification[value="translation"] button[anonid="translate"] {
color: #FBFBFB;
border: medium none;
background: none repeat scroll 0% 0% #0095DD;
border-radius: 2px;
font-size: 14px;
margin: 0px 29px 0px 0px;
height: 30px;
width: 120px;
}

notification[value="translation"] button[anonid="translate"]:hover {
background: none repeat scroll 0% 0% #008ACB;
}

notification[value="translation"] button[anonid="translate"]:active {
  background: none repeat scroll 0% 0% #008ACB;
}


5) The other buttons should have this CSS (again, hope this is the correct code for it):

notification[value="translation"] button {
border: 1px solid #C1C1C1;
background-color: #FBFBFB;
border-radius: 2px;
font-size: 14px;
margin: 0px 29px 0px 0px;
height: 30px;
width: 120px;
}

notification[value="translation"] button:hover {
background-color: #EBEBEB;
}

notification[value="translation"] button:active {
background-color: #EBEBEB;
}


Working on getting my Firefox builds going this afternoon so I can test and modify actual patches. Thanks for your patience in the meantime!
Attachment #8419692 - Flags: feedback?(sfranks)
(In reply to Sevaan Franks from comment #21)
> 1) Increase the height of the infobar to 40px
> 2) The listmenu background should be white, not the same colour as the
> infobar.
> 3) Buttons should have a height of 30px

the user may have changed the system fonts size, I don't think we should hardcode widths/heights and font-size in px. We can use relative measures though, like rem/ch. The same is valid for margin/padding or it would look bad with a different font size.
(In reply to Marco Bonardo [:mak] from comment #22)
> (In reply to Sevaan Franks from comment #21)
> > 1) Increase the height of the infobar to 40px
> > 2) The listmenu background should be white, not the same colour as the
> > infobar.
> > 3) Buttons should have a height of 30px
> 
> the user may have changed the system fonts size, I don't think we should
> hardcode widths/heights and font-size in px. We can use relative measures
> though, like rem/ch. The same is valid for margin/padding or it would look
> bad with a different font size.

Good point! In essence, the buttons should look like this: http://cl.ly/image/303q3Y45320K
Attached patch screenshot (obsolete) — Splinter Review
something like this?
I seem to remember we wanted to make this infobar less obtrusive as possible, doesn't a so tall infobar go against that requirement? I'm also a little bit unsure about the rhythm lost between the menulist and its popup contents.
Btw, I will now attach this version of the patch and let you play with it.
Attachment #8419692 - Attachment is obsolete: true
Attached image screenshot
I should actually attach a real screenshot :)
Attachment #8420451 - Attachment is obsolete: true
Attached patch patch v4 (obsolete) — Splinter Review
Attachment #8419641 - Attachment is obsolete: true
Attachment #8419641 - Flags: review?(mano)
Flags: needinfo?(sfranks)
What's up with the double arrow in the language picker? This appears to be Mac style. It's non-native on Windows. Surely if we need to use the same style across platforms (seems questionable), other platforms will have to follow Windows rather than the other way around, given where most of our users are?
(In reply to Dão Gottwald [:dao] from comment #27)
> What's up with the double arrow in the language picker?
> This appears to be Mac style. It's non-native on Windows.

It's in the mock-up, https://bug988478.bugzilla.mozilla.org/attachment.cgi?id=8400844
Sounds like the new unified style doesn't necessarily use native glyphs. But I'm not the best person to reply to this question.  Fwiw, in-content prefs already use this double arrow glyph across platforms for menulists (whether wanted or not, that's the current status)
(In reply to Marco Bonardo [:mak] from comment #28)
> (In reply to Dão Gottwald [:dao] from comment #27)
> > What's up with the double arrow in the language picker?
> > This appears to be Mac style. It's non-native on Windows.
> 
> It's in the mock-up,
> https://bug988478.bugzilla.mozilla.org/attachment.cgi?id=8400844
> Sounds like the new unified style doesn't necessarily use native glyphs. But
> I'm not the best person to reply to this question.  Fwiw, in-content prefs
> already use this double arrow glyph across platforms for menulists (whether
> wanted or not, that's the current status)

Newer mockups of ICUI prefs use the normal (not double arrow) dropdown.
(In reply to Tim Nguyen [:ntim] from comment #29)
> Newer mockups of ICUI prefs use the normal (not double arrow) dropdown.

Thanks for pointing that out, I'll wait for Sevaan's feedback and eventually update the styling where needed.
FWIW the first version of the Chameleon project site(UI unification) can be found here : http://people.mozilla.org/~jgruen/chameleon/ 

Button styling can be seen here : http://people.mozilla.org/~mmaslaney/incontent/Preferences%20-%20General@2x.png
Blocks: 988480
(In reply to Marco Bonardo [:mak] from comment #30)
> (In reply to Tim Nguyen [:ntim] from comment #29)
> > Newer mockups of ICUI prefs use the normal (not double arrow) dropdown.
> 
> Thanks for pointing that out, I'll wait for Sevaan's feedback and eventually
> update the styling where needed.

Hi Marco,

Looking good.

I've highlighted a couple changes...

1) The icon is too close to the right edge. It should be aligned under the back arrow for the most aesthetic placement.
2) The close icon is too close to the left edge, and should be aligned under the menu icon
3) We're getting rid of the double-arrow in the dropdown
4) The arrow in the "Options" button needs to be aligned more to the right
5) The dropdown arrows should be bigger.

Thanks!

Highlighted changes: http://cl.ly/image/0G0E0G2f1A2b
Flags: needinfo?(sfranks)
Whiteboard: [translation] p=5 s=it-32c-31a-30b.1 [qa-] → [translation] p=5 s=it-32c-31a-30b.2 [qa-]
(In reply to Sevaan Franks from comment #32)

> 1) The icon is too close to the right edge. It should be aligned under the
> back arrow for the most aesthetic placement.
> 2) The close icon is too close to the left edge, and should be aligned under
> the menu icon

These 2 placements are the same for all infobars, I don't think we should touch them here or even just for a single infobar. I'll file a follow-up to change them globally for all infobars and all themes (where applicable).

> 3) We're getting rid of the double-arrow in the dropdown

ok

> 4) The arrow in the "Options" button needs to be aligned more to the right

ok

> 5) The dropdown arrows should be bigger.

That is the native Windows glyph, I will see what we have around, IIRC the Linux theme has already a better glyph.
filed bug 1009501 for the infobar icon/close re-alignment.
Attached patch patch v5Splinter Review
This one addresses latest feedback, apart from the follow-up bug, and should be good for review. not attaching a screenshot cause it looks the same as comment 32 (again, apart from the follow-up)
Attachment #8420453 - Attachment is obsolete: true
Attachment #8421772 - Flags: review?(mano)
It's not clear to me why a lot of these styles need to be specific to the translation notification bar? Sevaan, is there a reason we can't apply the new colours and button styles to all notification bars (some parts only to notification bars of [type="info"])? Every time we diverge from a common theme, it makes maintenance harder and it's becoming an increasing trend recently.

I suggest that as much as possible be changed in https://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/notification.css except for the special aspects of translation such as the arrow (although arguably that should also be reusable).
Flags: needinfo?(sfranks)
(In reply to Matthew N. [:MattN] from comment #36)
> It's not clear to me why a lot of these styles need to be specific to the
> translation notification bar? Sevaan, is there a reason we can't apply the
> new colours and button styles to all notification bars (some parts only to
> notification bars of [type="info"])?

This was already explained in previous comments, this kind of notification is different from the existing ones since it requires a different kind of user interaction. Sure in future it may be (very easily) shared with other notifications, but doesn't make sense when the only consumer so far is an experimental infobar.

Plus UX team is working on project Chameleon that will define a unified style for Firefox, until that is ready it's really complicate to guess the right direction for the global style (let alone putting it into toolkit).
Attachment #8421772 - Flags: review?(mano) → review+
https://hg.mozilla.org/integration/fx-team/rev/c058132f270e
Target Milestone: --- → Firefox 32
sorry had to backout this change for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=40004525&tree=Fx-Team
(In reply to Carsten Book [:Tomcat] from comment #39)
> sorry had to backout this change for test failures like
> https://tbpl.mozilla.org/php/getParsedLog.php?id=40004525&tree=Fx-Team

This is because:

https://hg.mozilla.org/integration/fx-team/rev/c058132f270e#l3.15

looks like:

3.15 +@media (min-resolution: 1.25dppx) {
3.16 +  list-style-image: url(chrome://browser/skin/translation-16@2x.png);
3.17 +  -moz-image-region: rect(0px, 64px, 32px, 32px);
3.18 +}

so it needs a selector and then it should be fine.
re-landed with the fix Gijs suggested.  I ran the test locally to ensure it was passing (sounds like one needs mach package and --app-override="dist" to run that test).
https://hg.mozilla.org/integration/fx-team/rev/21f7015dfdb6

I'm clearing the needingo to Sevaan cause the answer was already here, once this becomes an actual feature UX may decide to make this one of the possible default styles of the infobar (provided other infobars will have the same interaction scheme) and a dedicated bug can take care of moving these few rules.
Flags: needinfo?(sfranks)
https://hg.mozilla.org/mozilla-central/rev/21f7015dfdb6
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Marco Bonardo [:mak] from comment #41)
> re-landed with the fix Gijs suggested.  I ran the test locally to ensure it
> was passing (sounds like one needs mach package and --app-override="dist" to
> run that test).
> https://hg.mozilla.org/integration/fx-team/rev/21f7015dfdb6

Just so I'm clear on this, did the test break/fail without ./mach package and --app-override dist ? AFAIK the end-result should be the same in both cases...
Flags: needinfo?(mak77)
(In reply to :Gijs Kruitbosch from comment #43)
> Just so I'm clear on this, did the test break/fail without ./mach package
> and --app-override dist ? AFAIK the end-result should be the same in both
> cases...

yes it was timing out
Flags: needinfo?(mak77)
Status: RESOLVED → VERIFIED
Depends on: 1025956
Regressions: 1605609
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: