Make the sidebar not suck

NEW
Unassigned

Status

SeaMonkey
Sidebar
14 years ago
7 years ago

People

(Reporter: Doron Rosenberg (IBM), Unassigned)

Tracking

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

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

14 years ago
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?

Comment 1

14 years ago
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.
(Reporter)

Comment 2

14 years ago
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.

Comment 3

14 years ago
Sure, but will people know that its the sidebar?
(Reporter)

Comment 4

14 years ago
Screenshot of what I have: http://nexgenmedia.net/sidebar-new.png
(Reporter)

Comment 5

14 years ago
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.
(Reporter)

Comment 6

14 years ago
Created attachment 141628 [details] [diff] [review]
Current state - a lot removed, some parts still broken.

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

Comment 7

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

Comment 8

14 years ago
> 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.
(Reporter)

Comment 10

14 years ago
Created attachment 142066 [details] [diff] [review]
All functionality is there

All functionality is now implemented, going to do some more cleaup and testing.
Attachment #141628 - Attachment is obsolete: true

Updated

14 years ago
Blocks: 57654
(Reporter)

Comment 11

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

(Reporter)

Comment 12

14 years ago
Created attachment 143416 [details] [diff] [review]
new patch
Assignee: sidebar → doronr
Attachment #142066 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Reporter)

Updated

14 years ago
Attachment #143416 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 13

14 years ago
Created attachment 148686 [details] [diff] [review]
Updated to work on trunk and to work better with older profiles
Attachment #143416 - Attachment is obsolete: true
(Reporter)

Comment 14

14 years ago
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)
(Reporter)

Updated

14 years ago
Attachment #143416 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 15

14 years ago
Created attachment 151022 [details] [diff] [review]
Fixes the issue with the search sidebar not opening if it was never created.
Attachment #148686 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #148686 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 16

14 years ago
Created attachment 151023 [details] [diff] [review]
Forgot to diff the .xul files

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

Updated

14 years ago
Attachment #151022 - Attachment is obsolete: true
(Reporter)

Updated

14 years ago
Attachment #151023 - Flags: review?(neil.parkwaycc.co.uk)

Comment 17

14 years ago
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
(Reporter)

Comment 18

14 years ago
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 19

14 years ago
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?
(Reporter)

Comment 20

14 years ago
Created attachment 151885 [details] [diff] [review]
Patch against trunk.

Patched against trunk.
Attachment #151023 - Attachment is obsolete: true

Comment 21

14 years ago
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 22

14 years ago
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 23

14 years ago
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
(Reporter)

Comment 25

13 years ago
Created attachment 155207 [details] [diff] [review]
fixed the nits, and removed a lot of useless CSS.  Made it work in modern, had to change one file (it had a black top border, moved that to css)
Attachment #151885 - Attachment is obsolete: true
(Reporter)

Comment 26

13 years ago
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.
(Reporter)

Comment 27

13 years ago
ok, this also breaks, as Neil found out, having different panels in composer and
navigator. argh.
(Reporter)

Comment 28

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

Comment 29

13 years ago
(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 30

13 years ago
Comment on attachment 151885 [details] [diff] [review]
Patch against trunk.

Hmm... this broke print preview too.
Product: Browser → Seamonkey

Comment 31

13 years ago
(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. :-(

Comment 32

13 years ago
We should try to make the effect of sidebar.num_tabs_in_view=1 nicer.
Doron,
Are you still working on this ?

Comment 34

8 years ago
unassigning this as doron doesn't reply.
Assignee: doronr → nobody
Status: ASSIGNED → NEW
QA Contact: sidebar

Updated

7 years ago
Depends on: 613971
You need to log in before you can comment on or make changes to this bug.