The sidebar currently is pretty complex, due to the multiple panels that can be shown in the sidebar. Its probably slowing the browser down as well, though there are no numbers to prove it. The idea - rather than having multiple panels shown and only one actually with content, just have the whole area show the sidebar content. The sidebar header can stay, though we would need a way to tell what panel is currently loaded. Perhaps have the dropwdown show the name of the loaded panel rather than "tabs". Another way would be similar to my MultiBar at http://devedge.netscape.com/toolbox/sidebars/ - a dropdown beneath the sidebar header with the close button. I have a simple version working, currently weeding out unneeded sidebar code. Should we keep the "loading..." progress?

So, would you have to remove the word "Sidebar", or would you put Sidebar X [History V] I don't think we should remove the progress information.

I'm thinking of --------------------- [ History V] X | but am open to any other ideas. Also, I am thinking of keeping the customize sidebar dialog as is for now, and put in in the dropdown of the names.

Sure, but will people know that its the sidebar?

Screenshot of what I have: http://nexgenmedia.net/sidebar-new.png

Another option is to keep the "tabs" dropdown as it is today, and keep the current sidebar header panel that contains the panel name. Both would use the same amount of space.

another question - do we want to keep the grippy in the slider or not?

BTW, while you're rewriting this, can you use the nsIDOMXULElement properties such as .id, .hidden and .collapsed wherever possible?

> do we want to keep the grippy in the slider or not? Yes (if you'd ask /me/ ;-)), because clicking the grippy to expand/minimize the sidebar without hiding it is quite handy IMO.

/me lobs out his .2 NOK: I'm sorry to say this, but I really like the old behavior opposed to the new one demonstrated in the screenshot. The problem is that I must make one extra click on the mouse before getting to my destination sidebar. Another small problem is that I can not see at once what sidebars I have configured into the profile. Anyway, I know that the old behavior had many hacks, and that there are savings from this. With the old behaviour, we also had problems with low resolutions. In any case, I think you should remove the "horizontal rule" between the "Sidebar" heading and the dropdown (perhaps keep as much space as there is now, however ?), to easier indicate that the dropdown is part of the sidebar interface, and not a seperate component. About the grippy, that is a hard question. If this was a new product, I'd say remove it, because the user can always add it back through the menus and F9, but since it's already in use, people may be accustomed to using the grippy ? Consistency is also good, and it's used in many other places in the Seamonkey interface.

All functionality is now implemented, going to do some more cleaup and testing.

Zipped up version of the new files at: http://www.nexgenmedia.net/bugs/sidebar.zip

Comment on attachment 148686 [details] [diff] [review] Updated to work on trunk and to work better with older profiles bugging neil.

Full files can be found at http://www.nexgenmedia.net/bugs/233874/

Neil needs to r= this before we can begin to move forward on all the accessibility problems in the sidebar (listed in meta bug 89148).

The idea is to make the sidebar simpler, which means better usability, less of a performance impact, and easier accessability (which this patch doesn't cover).

Comment on attachment 151023 [details] [diff] [review] Forgot to diff the .xul files I can't get the patch to apply. Conflict in navigator.js RevealSearchPanel(). That's changed recently. Doron, can you put up a new patch that applies to the trunk?

Patched against trunk.

Comment on attachment 151885 [details] [diff] [review] Patch against trunk. Feedback: * This solves a whole lot of accessibility problems with the sidebar. * There are some issues with this patch in the modern theme. Things don't look right, and the combo box changes size on mouseover. * There's a JS assertion when you type Alt+PgUp/Alt+PgDn, which use to navigate to the prev/next sidebar without changing focus (would be nice if it still did). * I'd like if there was a mnemonic accesskey for the combobox -- perhaps underline the _S_ in _S_idebar so that it's easy to jump there and switch tabs?

Comment on attachment 151885 [details] [diff] [review] Patch against trunk. > <binding id="sidebar-header-box" extends="xul:box"> > <content align="center"> You're not using this attribute any more... >- <xul:label class="sidebar-header-text" xbl:inherits="value=label,crop" crop="right" flex="1"/> So now you can remove the label attribute from the sidebarheader... >- <xul:box> >+ <xul:vbox flex="1"> > <children/> >- </xul:box> >+ </xul:vbox> I think the box was only here to keep the children away from the label... you can use CSS or orient="vertical" on the sidebarheader instead.

Comment on attachment 151885 [details] [diff] [review] Patch against trunk. >+ <sidebarheader id="sidebar-header" class="sidebarheader-main" >+ type="box" label="&sidebar.panels.label;" >+ onmouseup="PersistHeight();" prefixhidden="true" >+ tooltipopen="&sidebar.open.tooltip;" >+ tooltipclose="&sidebar.close.tooltip;"> As I mentioned, you don't need the label. >+ <hbox flex="1"> This needs the align="center" from the previous comment. > <menuitem label="&sidebar.customize.label;" accesskey="&sidebar.customize.accesskey;" No accesskeys on menulist menuitems, please. >+ <template> >+ <rule> >+ <conditions> >+ <content uri="?uri"/> >+ <triple subject="?uri" >+ predicate="http://home.netscape.com/NC-rdf#panel-list" >+ object="?panel-list"/> >+ <member container="?panel-list" child="?panel"/> >+ <triple subject="?panel" >+ predicate="http://home.netscape.com/NC-rdf#title" >+ object="?title" /> >+ <triple subject="?panel" >+ predicate="http://home.netscape.com/NC-rdf#content" >+ object="?content" /> >+ </conditions> >+ <action> >+ <menupopup> >+ <menuitem uri="?panel" type="checkbox" >+ label="?title" url="?content" /> >+ </menupopup> >+ </action> >+ </rule> >+ </template> Would the "simple" template syntax be simpler? Also, rather than url="?content" I would prefer value="?content". >+ <spacer flex="1"/> The old spacer had a flex of 100%. The % is actually ignored, thus making the spacer take up most of the space. So much, in fact, that I doubt there's any point making the hbox flex, thus making the spacer superfluous. >+ hidden="true" collapsed="true" content="?content"/> This is no longer in a template.

Doron: Did you notice comment 21, 22, 23 ?

Note that composer's sidebar is hosed a bit, that is due to the <command> not overlaying. Feel free to review though while I sort that out.

ok, this also breaks, as Neil found out, having different panels in composer and navigator. argh.

Do we want to allow the apps to have different sidebar panels selected?

(In reply to comment #28) > Do we want to allow the apps to have different sidebar panels selected? If we can do it now, yes. If we can't do it now, no.

Comment on attachment 151885 [details] [diff] [review] Patch against trunk. Hmm... this broke print preview too.

(In reply to comment #4) > Screenshot of what I have: http://nexgenmedia.net/sidebar-new.png I'd like to suggest removing the "Sidebar" text and moving the drop-down to the top line. This would save an entire row by removing (somewhat) useless clutter. Everybody will see that it is a sidebar (does the Bookmarks Toolbar say "Bookmarks Toolbar" somewhere? No!). Well, I'm using Thunderbird, so feel free to ignore my suggestion. ;-) Anyhow, great job guys. This bug is very similar to the one I filed on Thunderbird (bug 209642) - which was WONTFIXed. :-(

We should try to make the effect of sidebar.num_tabs_in_view=1 nicer.

Doron, Are you still working on this ?

unassigning this as doron doesn't reply.