960198 - No obvious option to unhide SocialAPI sidebar

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Christian Ascheberg]

STR:
1. enable Facebook SocialAPI
2. in SocialAPI sidebar: click gear icon -> uncheck "show sidebar" (or click "Turn off [...]")

Result:
- only the "Share" button is now visible, there is no dedicated Facebook button (if turned off: there is no "Services" button)
The only way I found to unhide the sidebar on Windows is to press the Alt-key and use the "View" menu, but this option is probably unknown to most users.

Comment 1

[u279076]

Thanks for the bug report, Christian. I confirm this is a problem. It might be a UX issue more than an engineering bug but I'll let the SocialAPI team decide on that.

Comment 2

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Dirty sketch: showing notification where the hidden toolbar once was

The sidebar can also be unhidden from the sidebars menu within Firefox, but yep, not very discoverable - this is part of the challenge that comes with removing UI elements.  While showing something permanently in the Firefox toolbar is a non-goal, we could address this by giving the user a notification such as "your sidebar has been hidden, to get it back go to ..."  

It could have an undo option and a "don't show again" button.  Would probably have to float in content (under where the sidebar was) rather than at the browser level, though.  That may be very technically difficult, but it would help the problem.  What do you think that would cost for developer/implementation pain, Shane?

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: new customizable widget for sidebar selection

based on irc conversation with @Boris;

- customizable widget that contains the same elements as view->sidebar
- only show widget if social providers (with sidebars) are installed, otherwise leave current ux as-is

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: buttonInPanel.png

Just using placeholder images for right now.

This is the button in the customization/menu panel

Comment 5

[Shane Caraveo (:mixedpuppy)]

Attachment: buttonPanelShowing.png

panel now showing the sidebar options that are avialable

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: toolbarButton.png

button has been moved by user to toolbar

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8368357 [details] [diff] [review]
new customizable widget for sidebar selection

Review of attachment 8368357 [details] [diff] [review]:
-----------------------------------------------------------------

I think the approach is sound, but just some minor concerns.

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +291,5 @@
> +        let win = doc.defaultView;
> +        let Social = win.Social;
> +        let providers = Social && Social.providers && Social.providers.some(p => p.sidebarURL);
> +        if (!providers) {
> +          node.setAttribute("hidden", "true");

Not sure how I feel about this hidey-showy behaviour. I think I'd be more inclined to support enabled / disabled, simply because I imagine it would feel spooky if the user were to move an item to some area, and then have it disappear (or suddenly appear out of nowhere).

@@ +305,5 @@
> +            return;
> +          Services.obs.removeObserver(updateState, "social:providers-changed");
> +        }.bind(this),
> +        onWidgetInstanceRemoved: function(aWidgetId, aDoc) {
> +          if (aWidgetId != this.id || aDoc != aDoc)

aDoc != aDoc will always return false.

Comment 9

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> - only show widget if social providers (with sidebars) are installed,
> otherwise leave current ux as-is

(In reply to Mike Conley (:mconley) from comment #8)
> Not sure how I feel about this hidey-showy behaviour. I think I'd be more
> inclined to support enabled / disabled, simply because I imagine it would
> feel spooky if the user were to move an item to some area, and then have it
> disappear (or suddenly appear out of nowhere).

Food for thought: this would become the only visible UI-based way (other than the view menu, which isn't visible by default) to toggle the native sidebars (bookmarks, history).

If a user gets used to / actually needs this widget for this goal, and for some reason decides he no longer needs (or never needed) any social provider and removes them (I don't even know if this is possible, but I'm considering this scenario, since the code seems to do the same), then they would lose this ability as well.

The bookmarks/history(/any other?) sidebar toggler shouldn't depend on there being any social providers. My suggestion: either make the widget always visible, with maybe a string label saying "No social providers available" below the other sidebar options if that's the case, or don't include them at all; I think consistency should be kept in both with and without social providers scenarios.

Comment 10

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Luís Miguel [:Quicksaver] from comment #9)
> or don't include them at all

By "them" I meant other sidebar options.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Luís Miguel [:Quicksaver] from comment #9)
> If a user gets used to / actually needs this widget for this goal, and for
> some reason decides he no longer needs (or never needed) any social provider
> and removes them (I don't even know if this is possible, but I'm considering
> this scenario, since the code seems to do the same), then they would lose
> this ability as well.

That's a very good point - I hadn't realized that. Thanks for mentioning! :)

Comment 12

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarToolbarbutton.png

Comment 13

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarMenuPanel.png

Comment 14

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarMenuButton.png

Comment 15

[Shane Caraveo (:mixedpuppy)]

Attachment: new customizable widget for sidebar selection

asking for a review, though pending ui approval(s) for any checkin

Comment 16

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarMenuButton.png

Comment 17

[Shane Caraveo (:mixedpuppy)]

@Boriss I went ahead an removed the hidding of the button if there are no social providers, making it a generic sidebar button.  It's easy to add that hidding back.  Let me know what would be preferable here.

Comment 18

[Jennifer Morrow [:Boriss] (UX)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #17)
> @Boriss I went ahead an removed the hidding of the button if there are no
> social providers, making it a generic sidebar button.  It's easy to add that
> hidding back.  Let me know what would be preferable here.

Sigh, tough call.  Good point in Comment 9, Luis.  

Pros of hiding: Not cluttering the UI further for non-SocialAPI users, which is the vast majority
Cons of hiding: Inconsistency - visible UI that affects non-SocialAPI being dependent on SocialAPI status.

As much as adding more stuff with meager user value to the customize menu is a non-goal, I think you're  right.  Particularly since we put things like Character Encoding in the "More Tools" section already, and potentially this appeals to a wider demographic than character encoding.  Removal of hiding is ok.

Shane, for review - can you confirm that the Sidebars button would appear in "More Tools to Add to the Menu and Toolbar (http://cl.ly/image/1T2v3w2p3r3n)," not the default customize menu?  The attachments show it in the main Customize menu (http://cl.ly/image/3Z3a400J1d3O).

Comment 19

[Shane Caraveo (:mixedpuppy)]

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #18)

> Shane, for review - can you confirm that the Sidebars button would appear in
> "More Tools to Add to the Menu and Toolbar
> (http://cl.ly/image/1T2v3w2p3r3n)," not the default customize menu?  The
> attachments show it in the main Customize menu
> (http://cl.ly/image/3Z3a400J1d3O).

It can be easily changed to do that, I'll update the patch.

Comment 20

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebar button

button lives in "more tools" by default.

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Haven't forgotten about this - will do a review tomorrow.

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8376623 [details] [diff] [review]
sidebar button

Review of attachment 8376623 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me - but please remove the erroneous documentation.

Thanks Shane! I think this will make our sidebar users pretty happy.

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +306,5 @@
>  
>        parent.appendChild(items);
>      }
>    }, {
> +    // while this button provides access to all sidebars, our primary use case

This comment no longer applies.

Comment 23

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebar button

comment removed per review feedback.

Comment 24

[Mike Conley (:mconley) (:⚙️)]

Just a note:

(In reply to Luís Miguel [:Quicksaver] from comment #9)

> Food for thought: this would become the only visible UI-based way (other
> than the view menu, which isn't visible by default) to toggle the native
> sidebars (bookmarks, history).

This is not actually true anymore - both the Bookmarks and History widgets have sidebar toggles in their menus.

I just noticed that today. Not sure that changes anything here, but thought I'd mention it.

Comment 25

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike Conley (:mconley) from comment #24)
> Just a note:
> 
> (In reply to Luís Miguel [:Quicksaver] from comment #9)
> 
> > Food for thought: this would become the only visible UI-based way (other
> > than the view menu, which isn't visible by default) to toggle the native
> > sidebars (bookmarks, history).
> 
> This is not actually true anymore - both the Bookmarks and History widgets
> have sidebar toggles in their menus.
> 
> I just noticed that today. Not sure that changes anything here, but thought
> I'd mention it.

Yeah, in the unmodified case this button isn't useful, but there are a lot of sidebar addons that will benefit from this, as well as the social provider addons.

Comment 26

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Mike Conley (:mconley) from comment #24)
> This is not actually true anymore - both the Bookmarks and History widgets
> have sidebar toggles in their menus.
> 
> I just noticed that today. Not sure that changes anything here, but thought
> I'd mention it.

True, although I agree with comment 25 and I think my initial point still stands. :)

Comment 27

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Diagram: Differences in visual styling between patch and Australis that should be addressed

(In reply to Shane Caraveo (:mixedpuppy) from comment #23)
> Created attachment 8382261 [details] [diff] [review]
> sidebar button
> 
> comment removed per review feedback.

This is looking good, but a few visual tweaks make it visually unlike the rest of Australis.  Shane, are the styles that govern the rest of the browser not available here?  Seems like a lot of work to try and replicate the styles in the panels.

Comment 28

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarToolbarbuttonCurrent.png

current nightly version of toolbarbutton menu, some stuff is fixed by nightly.

- font is bigger
- checkmark is different

note: developer tools in the toolbar has the same issues

Comment 29

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Diagram: Comparison of Nightly devtools with attachment 8382649

(In reply to Shane Caraveo (:mixedpuppy) from comment #28)
> Created attachment 8382649 [details]
> sidebarToolbarbuttonCurrent.png

Still seeing a couple issues - devtools is a good comparison, this should display identically as far as type/style

Comment 30

[Shane Caraveo (:mixedpuppy)]

ok, font differences are explained by bug 931040

Comment 31

[Jennifer Morrow [:Boriss] (UX)]

Comment on attachment 8376519 [details]
sidebarMenuPanel.png

Note: Text and style match devtools menu, but don't appear to in this screenshot because of regression in bug 931040

Comment 32

[Jennifer Morrow [:Boriss] (UX)]

Comment on attachment 8382649 [details]
sidebarToolbarbuttonCurrent.png

Note: Text and style match devtools menu, but don't appear to in this screenshot because of regression in bug 931040

Comment 33

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8376623 [details] [diff] [review]
sidebar button

obsoleted by attachment 8382261 [details] [diff] [review]

Comment 34

[Shane Caraveo (:mixedpuppy)]

https://tbpl.mozilla.org/?tree=Try&rev=6b8f95e1d004

Comment 35

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/fx-team/rev/29c4b76e7364

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/29c4b76e7364

Comment 37

[Mike de Boer [:mikedeboer]]

I think we should back this out due to the regression filed as bug 978475.

I'd be up for reviewing a patch with the correct icons in the correct files.

Comment 38

[Tim Nguyen :ntim]

and also fix bug 978491 at the same time (also icon related).

Comment 39

[Tim Nguyen :ntim]

btw, on mac, the icon should be blue when the panel is opened.

Comment 40

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mike de Boer [:mikedeboer] from comment #37)
> I think we should back this out due to the regression filed as bug 978475.

I agree.

Comment 41

[Shane Caraveo (:mixedpuppy)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #40)
> (In reply to Mike de Boer [:mikedeboer] from comment #37)
> > I think we should back this out due to the regression filed as bug 978475.
> 
> I agree.

can we keep the strings in so I could uplift the button to aurora after merge?

Comment 42

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Backout except string changes

Comment 43

[:Gijs (he/him)]

Comment on attachment 8384646 [details] [diff] [review]
Backout except string changes

Review of attachment 8384646 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 44

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/77843e2f07a4

Comment 45

[Ryan VanderMeulen [:RyanVM]]

Merge of backout.
https://hg.mozilla.org/mozilla-central/rev/77843e2f07a4

Comment 46

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarimages patch

modifications to image files for sidebar button, includes inverted images that were missing before.  tested on win7, osx 10.9, linux.  not tested on hidpi osx, or any alternate windows themes (aero?)

Comment 47

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebar button patch

Comment 48

[Shane Caraveo (:mixedpuppy)]

Attachment: osx-menupanel.png

Comment 49

[Shane Caraveo (:mixedpuppy)]

Attachment: osx-customization.png

Comment 50

[Shane Caraveo (:mixedpuppy)]

Attachment: osx-toolbarmenu.png

Comment 51

[Shane Caraveo (:mixedpuppy)]

Attachment: osx-menupanel-open.png

Comment 52

[Shane Caraveo (:mixedpuppy)]

Attachment: win-menupanel-open.png

Comment 53

[Shane Caraveo (:mixedpuppy)]

Attachment: linux-menupanel-open.png

Comment 55

[Shane Caraveo (:mixedpuppy)]

note, I was unable to reproduce bug 978484 on win7, not sure if it is winxp specific.

Comment 56

[Tim Nguyen :ntim]

(In reply to Shane Caraveo (:mixedpuppy) from comment #52)
> Created attachment 8387973 [details]
> win-menupanel-open.png

Unrelated, but what's that G+ provider ?

Comment 57

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #56)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #52)
> > Created attachment 8387973 [details]
> > win-menupanel-open.png
> 
> Unrelated, but what's that G+ provider ?

Nevermind, found it.

Comment 58

[:Gijs (he/him)]

Comment on attachment 8387968 [details] [diff] [review]
sidebar button patch

Review of attachment 8387968 [details] [diff] [review]:
-----------------------------------------------------------------

f+ because of all the generic subviewbutton rules on Windows/Linux which are very confusing. Why did you add those?

::: browser/components/customizableui/content/panelUI.inc.xul
@@ +154,5 @@
>      </panelview>
>  
> +    <panelview id="PanelUI-sidebar" flex="1">
> +      <label value="&appMenuSidebars.label;" class="panel-subview-header"/>
> +      <vbox id="PanelUI-sidebarItems"/>

This needs a panel-subview-body class.

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +337,5 @@
> +
> +      // First clear any existing menuitems then populate. Social sidebar
> +      // options may not have been added yet, so we do that here. Add it to the
> +      // standard menu first, then copy all sidebar options to the panel.
> +      win.SocialSidebar.clearProviderMenus();

Out of interest, does this do what it says on the tin and clear all the menus, and then only repopulate one of them below by calling _populateProviderMenu? That seems off... but maybe I'm confused?

@@ +340,5 @@
> +      // standard menu first, then copy all sidebar options to the panel.
> +      win.SocialSidebar.clearProviderMenus();
> +      let providerMenuSeps = menu.getElementsByClassName("social-provider-menu");
> +      if (providerMenuSeps.length > 0)
> +        win.SocialSidebar._populateProviderMenu(providerMenuSeps[0]);

Nit: by convention this is a private method. If this needs to be exposed, then that should be done. :-)

@@ +374,5 @@
> +      items.appendChild(fragment);
> +    },
> +    onViewHiding: function(aEvent) {
> +      let doc = aEvent.target.ownerDocument;
> +      let win = doc.defaultView;

Nit: you don't use |win| in this function.

::: browser/themes/shared/toolbarbuttons.inc.css
@@ +69,5 @@
>  }
>  
> +#sidebar-button[cui-areatype="toolbar"] {
> +  -moz-image-region: rect(0, 684px, 18px, 666px);
> +}

Please move this and the email-link button to the end while we're here, so that the regions are sorted. :-)

::: browser/themes/windows/customizableui/panelUIOverlay.css
@@ +46,5 @@
>  .subviewbutton.bookmark-item > .toolbarbutton-icon {
>    -moz-margin-start: 3px;
>  }
>  
> +.subviewbutton[image]:not([checked="true"]) > .toolbarbutton-text {

These rules shouldn't be necessary. Ditto for the Linux ones. Can you explain why you added them?

Comment 59

[Tim Nguyen :ntim]

(In reply to :Gijs Kruitbosch from comment #58) 
> ::: browser/themes/windows/customizableui/panelUIOverlay.css
> @@ +46,5 @@
> >  .subviewbutton.bookmark-item > .toolbarbutton-icon {
> >    -moz-margin-start: 3px;
> >  }
> >  
> > +.subviewbutton[image]:not([checked="true"]) > .toolbarbutton-text {
> 
> These rules shouldn't be necessary. Ditto for the Linux ones. Can you
> explain why you added them?
This is because Windows and Linux hide the icon when an item is checked. While Mac doesn't do it.

Comment 60

[Tim Nguyen :ntim]

Attachment: Screenshot of menu on Windows to explain the css rules

Comment 61

[:Gijs (he/him)]

(In reply to Tim Nguyen [:ntim] from comment #59)
> (In reply to :Gijs Kruitbosch from comment #58) 
> > ::: browser/themes/windows/customizableui/panelUIOverlay.css
> > @@ +46,5 @@
> > >  .subviewbutton.bookmark-item > .toolbarbutton-icon {
> > >    -moz-margin-start: 3px;
> > >  }
> > >  
> > > +.subviewbutton[image]:not([checked="true"]) > .toolbarbutton-text {
> > 
> > These rules shouldn't be necessary. Ditto for the Linux ones. Can you
> > explain why you added them?
> This is because Windows and Linux hide the icon when an item is checked.
> While Mac doesn't do it.

That doesn't nearly explain half the rules, like why the icon needs to be sized for all buttons, not just sidebar ones, and why the padding on the icon needs to change on all buttons, too. Blake just fixed a whole bunch of this stuff, I'm not keen to change it without good reasons. I'm sure Shane had some, so I'd like to hear them from him.

Comment 62

[Shane Caraveo (:mixedpuppy)]

(In reply to :Gijs Kruitbosch from comment #61)
> (In reply to Tim Nguyen [:ntim] from comment #59)
> > (In reply to :Gijs Kruitbosch from comment #58) 
> > > ::: browser/themes/windows/customizableui/panelUIOverlay.css
> > > @@ +46,5 @@
> > > >  .subviewbutton.bookmark-item > .toolbarbutton-icon {
> > > >    -moz-margin-start: 3px;
> > > >  }
> > > >  
> > > > +.subviewbutton[image]:not([checked="true"]) > .toolbarbutton-text {
> > > 
> > > These rules shouldn't be necessary. Ditto for the Linux ones. Can you
> > > explain why you added them?
> > This is because Windows and Linux hide the icon when an item is checked.
> > While Mac doesn't do it.
> 
> That doesn't nearly explain half the rules, like why the icon needs to be
> sized for all buttons, not just sidebar ones, and why the padding on the
> icon needs to change on all buttons, too. Blake just fixed a whole bunch of
> this stuff, I'm not keen to change it without good reasons. I'm sure Shane
> had some, so I'd like to hear them from him.

Any panel that ends up using both checkbox and image will end up needing this css to have the correct appearance, primarily for the showing/hidding of images vs. checkmarks on windows and linux.  The specific size are to deal with external images that may not conform to the size we need.  I got the stuff looking right across platforms, but if there is a better way to do this I'm happy to change it.

If you would be more comfortable with it, we can add #PanelUI-sidebar and limit it to the sidebar panel, but eventually someone else will stumble across this same issue.

Comment 63

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarbtn patch

this appears fine on all platforms.

Comment 64

[:Gijs (he/him)]

Comment on attachment 8388703 [details] [diff] [review]
sidebarbtn patch

Review of attachment 8388703 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Note that you'll race for bitrot with bug 959640 for the icon range insertion bits... well, as soon as that bug has a patch, anyhow. :-)

::: browser/themes/linux/customizableui/panelUIOverlay.css
@@ +36,5 @@
> +  width: 16px;
> +  height: 16px;
> +}
> +
> +.subviewbutton[image][checked="true"] > .toolbarbutton-icon {

Nit: this should probably be using the same -moz-any as the rule that sets -moz-padding-start. :-)

(won't make any difference now, but it'll make it clearer that one relies on the other to work well, future-proofs it, etc.)

::: browser/themes/windows/customizableui/panelUIOverlay.css
@@ +54,5 @@
> +  width: 16px;
> +  height: 16px;
> +}
> +
> +.subviewbutton[image][checked="true"] > .toolbarbutton-icon {

Ditto.

Comment 65

[:Gijs (he/him)]

Attachment: images for sidebar button

Looks good, but the images weren't optimized. Fixed that.

Comment 66

[:Gijs (he/him)]

Comment on attachment 8388845 [details] [diff] [review]
images for sidebar button

Carrying over my own r+...

Comment 67

[Shane Caraveo (:mixedpuppy)]

Attachment: sidebarbtn patch

implement r+ comment, seems to work fine

Comment 68

[Shane Caraveo (:mixedpuppy)]

images: https://hg.mozilla.org/integration/fx-team/rev/d5799bfd78ab
patch:  https://hg.mozilla.org/integration/fx-team/rev/54edfc4fa73b

Comment 69

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d5799bfd78ab
https://hg.mozilla.org/mozilla-central/rev/54edfc4fa73b

Comment 70

[:Gijs (he/him)]

Can't uplift this because of strings.

Comment 71

[Andrei Vaida [:avaida]]

It is my understanding that the new "Sidebars" menu (and sub-menu) as well as the toolbar button should be displayed in a way that is similar to other menus/toolbars (e.g. developer tools). 

As you can see in this google doc [1], both the menu and toolbar show different hover and enabled state effects from one platform to another, here are a few notes to go along with the screenshots available in the doc:
 a. Windows 7 64-bit: the hovered and enabled effects go somewhat in the lines of aero, compared to the other menus or toolbars (e.g. developer tools). Also, the selected/hover rectangles overlap the right margin of the menu displayed on toolbar button click.
 b. Mac OS X 10.9: you literally can't tell which of the services is enabled (or hovered for that matter) from the menu panel or toolbar, in both of the screenshots Saavn is enabled and Delicious is hovered.
 c. Windows 8 64-bit: hover and enabled states are OS-specific, the same as the case of Windows 7 64-bit.
 d. Ubuntu 14.04 LTS 32-bit: the same as above.


So I guess that what I'm actually asking here is how should these menus or toolbar buttons be displayed? The same as other buttons/menus from Australis or in accordance with the OS?


1. https://docs.google.com/document/d/1ZJxifqQYY1QILombEjlSdFDRABv3h2KvC4a0UGupQSc/edit?usp=sharing

Comment 72

[:Gijs (he/him)]

(In reply to Andrei Vaida, QA [:avaida] from comment #71)
> It is my understanding that the new "Sidebars" menu (and sub-menu) as well
> as the toolbar button should be displayed in a way that is similar to other
> menus/toolbars (e.g. developer tools). 
> 
> As you can see in this google doc [1], both the menu and toolbar show
> different hover and enabled state effects from one platform to another, here
> are a few notes to go along with the screenshots available in the doc:
>  a. Windows 7 64-bit: the hovered and enabled effects go somewhat in the
> lines of aero, compared to the other menus or toolbars (e.g. developer
> tools). Also, the selected/hover rectangles overlap the right margin of the
> menu displayed on toolbar button click.
>  b. Mac OS X 10.9: you literally can't tell which of the services is enabled
> (or hovered for that matter) from the menu panel or toolbar, in both of the
> screenshots Saavn is enabled and Delicious is hovered.
>  c. Windows 8 64-bit: hover and enabled states are OS-specific, the same as
> the case of Windows 7 64-bit.
>  d. Ubuntu 14.04 LTS 32-bit: the same as above.
> 
> 
> So I guess that what I'm actually asking here is how should these menus or
> toolbar buttons be displayed? The same as other buttons/menus from Australis
> or in accordance with the OS?
> 
> 
> 1.
> https://docs.google.com/document/d/
> 1ZJxifqQYY1QILombEjlSdFDRABv3h2KvC4a0UGupQSc/edit?usp=sharing

You should file a new bug about this, and needinfo Shane there.

Comment 73

[Tim Nguyen :ntim]

(In reply to Andrei Vaida, QA [:avaida] from comment #71)
> It is my understanding that the new "Sidebars" menu (and sub-menu) as well
> as the toolbar button should be displayed in a way that is similar to other
> menus/toolbars (e.g. developer tools). 
> 
> As you can see in this google doc [1], both the menu and toolbar show
> different hover and enabled state effects from one platform to another, here
> are a few notes to go along with the screenshots available in the doc:
>  a. Windows 7 64-bit: the hovered and enabled effects go somewhat in the
> lines of aero, compared to the other menus or toolbars (e.g. developer
> tools). Also, the selected/hover rectangles overlap the right margin of the
> menu displayed on toolbar button click.
>  b. Mac OS X 10.9: you literally can't tell which of the services is enabled
> (or hovered for that matter) from the menu panel or toolbar, in both of the
> screenshots Saavn is enabled and Delicious is hovered.
>  c. Windows 8 64-bit: hover and enabled states are OS-specific, the same as
> the case of Windows 7 64-bit.
>  d. Ubuntu 14.04 LTS 32-bit: the same as above.
> 
> 
> So I guess that what I'm actually asking here is how should these menus or
> toolbar buttons be displayed? The same as other buttons/menus from Australis
> or in accordance with the OS?
> 
> 
> 1.
> https://docs.google.com/document/d/
> 1ZJxifqQYY1QILombEjlSdFDRABv3h2KvC4a0UGupQSc/edit?usp=sharing
That is bug 989751

Comment 74

[Andrei Vaida [:avaida]]

(In reply to Tim Nguyen [:ntim] from comment #73)
> That is bug 989751
That seems to be the case, thanks Tim!

Since both the Sidebars menu and toolbar work as expected in terms of functionality and the issues depicted in Comment 71 are already reported in Bug 989751, I'm going to mark this verified fixed.

Comment 75

[Matthew N. [:MattN]]

This bug adds a sidebar button that gives easier access to social, bookmark, & history sidebars.

Comment 76

[Lukas Blakk [:lsblakk] use ?needinfo]

Added to FF30 desktop release notes as: "New Sidebar button gives easier access to social, bookmark, & history sidebars"