Open Bug 233874 Opened 20 years ago Updated 14 years ago

Make the sidebar not suck

Categories

(SeaMonkey :: Sidebar, defect)

defect
Not set
normal

Tracking

(Not tracked)

People

(Reporter: doronr, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 7 obsolete files)

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.
Attached patch All functionality is there (obsolete) — Splinter Review
All functionality is now implemented, going to do some more cleaup and testing.
Attachment #141628 - Attachment is obsolete: true
Blocks: 57654
Zipped up version of the new files at:
  http://www.nexgenmedia.net/bugs/sidebar.zip

Attached patch new patch (obsolete) — Splinter Review
Assignee: sidebar → doronr
Attachment #142066 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #143416 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #143416 - Attachment is obsolete: true
Comment on attachment 148686 [details] [diff] [review]
Updated to work on trunk and to work better with older profiles

bugging neil.
Attachment #148686 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #143416 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #148686 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch Forgot to diff the .xul files (obsolete) — Splinter Review
Full files can be found at http://www.nexgenmedia.net/bugs/233874/
Attachment #151022 - Attachment is obsolete: true
Attachment #151023 - Flags: review?(neil.parkwaycc.co.uk)
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).
Blocks: 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?
Attached patch Patch against trunk. (obsolete) — Splinter Review
Patched against trunk.
Attachment #151023 - Attachment is obsolete: true
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.
Attachment #151023 - Flags: review?(neil.parkwaycc.co.uk)
Doron:
Did you notice comment 21, 22, 23 ?
Hardware: PC → All
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.
Product: Browser → Seamonkey
(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.
Assignee: doronr → nobody
Status: ASSIGNED → NEW
QA Contact: sidebar
Depends on: 613971
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: