403140 - Splitters in the bookmarks organizer should not be collapsible / auto size height of details pane

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

The splitters in the bookmarks organizer should not be collapsable (clicking on a splitter causes the window pane to minimize to the side of the window).  The splitters should not contain any type of visual affordance for gripping, for visual integration with the operating system (the purple seems curiously similar to Java Swing).

See bug# 393514 for a mockup.

Comment 1

[Alex Faaborg [:faaborg] (Firefox UX)]

Additional rationale:

-this isn't native to any platform (I'm pretty sure)
-it is easy to accidently collapse them when trying to drag them
-the purple grippy appearance is java-swing-style ugly

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

These 1990s era UI elements has gots to go.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: screenshot

something like this?
i've done some experimental change:
- fixed the back-forward buttons (hover, pressed state, padding)
- removed the grippy
- added min/max width values to panes
- changed the splitter to a single line
- set an initial height of 140px for details pane, enough for not having scrollbars with the more pane closed (making it enough for the details open is too much, takes about half of the window)

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: experimental changes

like above, plus light blue splitters like in Alex mockup (is this for vista only or for xp too?)... i could also add the translucent background to tree column headers if you want to try that

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

>light blue splitters like in Alex mockup (is this for vista
>only or for xp too?)

We should only hard code the color on Vista (and only if we are going to also hard code the window background color).

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

cleaned up a bit
- fixed back-forward buttons
- toolbar has min-height 30px, Vista toolbars are 30px
- removed the grippy
- added min/max width values to panes
- changed the splitter to a single line
- set an initial details panel height of 150px

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: screenshot on Vista

appearance on Vista after removal of grippy and fixed back-forward

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 312047 [details] [diff] [review]
patch

i can probably fix the datails pane to adapt height based on contents

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Beltzner, if i change the details pane to dynamically change its height based on content then persisting the user height makes no sense, on each select the panel height could change, on the other way if i persist the user height i cannot change the panel height and we have scrollbars. What is the correct behaviour?

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

We should probably spin off other changes to separate bugs, or adjust the summary of this bug (although I'm reluctant to do that since it is set as blocking).

>if i change the details pane to dynamically change its height based
>on content then persisting the user height makes no sense, on each select the
>panel height could change

On irc I instructed marco that we want to try to avoid ever having scroll bars, and as a secondary goal try not to have too much extra wasted space (for instance the area should get smaller when you hit "less".  Making the details area not user resizable (so it doesn't keep changing on them) is fine.

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

Alex, this is for your testing. I've done the changes we talked about yesterday, details pane is not resizable, but its height is calculated when the user select an item, deselect everything, or click on More/Less expander.
Plus previous fixes

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 312236 [details] [diff] [review]
patch

Personally I'm in favor of the change, but switching the ui-r to beltzner for his feedback.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 312236 [details] [diff] [review]
patch

I think we should be styling the splitter consistently with the browser sidebar splitter in the default css; to style that splitter appropriately for Vista we should use bug 403138

uir+ with that nit addressed

Comment 14

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

Reverted splitter style as requested.

fixed here:
- removed grippy from splitters
- winstripe left splitter styled like sidebar splitter
- winstripe back-forward buttons have correct styling now
- panels are not collapsable (added max/min sizes)
- details pane not resizable (auto-size based on contents and less/more)

Comment 15

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: screenshot on XP

Comment 16

[Asaf Romano (gone)]

Looks like the wrong file.

Comment 17

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

:\ the correct one

Comment 18

[Asaf Romano (gone)]

I think we could just remove the scrollbox and don't set any heights.

Comment 19

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

scrollbox gone

Comment 20

[Asaf Romano (gone)]

Comment on attachment 314448 [details] [diff] [review]
patch

r=mano

Comment 21

[Dietrich Ayala (:dietrich)]

Checking in browser/components/places/content/organizer.css;
/cvsroot/mozilla/browser/components/places/content/organizer.css,v  <--  organizer.css
new revision: 1.7; previous revision: 1.6
done
Checking in browser/components/places/content/places.js;
/cvsroot/mozilla/browser/components/places/content/places.js,v  <--  places.js
new revision: 1.154; previous revision: 1.153
done
Checking in browser/components/places/content/places.xul;
/cvsroot/mozilla/browser/components/places/content/places.xul,v  <--  places.xul
new revision: 1.129; previous revision: 1.128
done
Checking in browser/themes/gnomestripe/browser/places/organizer.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/places/organizer.css,v  <--  organizer.css
new revision: 1.14; previous revision: 1.13
done
Checking in browser/themes/pinstripe/browser/places/organizer.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/places/organizer.css,v  <--  organizer.css
new revision: 1.11; previous revision: 1.10
done
Checking in browser/themes/winstripe/browser/places/organizer.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/places/organizer.css,v  <--  organizer.css
new revision: 1.9; previous revision: 1.8
done

Comment 22

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

about the infoPane, the height could still be set up in the localstore.rdf for it, it should shrink when nothing is selected to leave some more space, but if the height is set up there it does not shrink.

Should we change the infoPane id or annotate this for beta users?

Comment 23

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

filled bug 428020 on that

Comment 24

[Thomas Stache]

(In reply to comment #22)
> about the infoPane, the height could still be set up in the localstore.rdf for
> it, it should shrink when nothing is selected to leave some more space, but if
> the height is set up there it does not shrink.

Thanks for the tip, it improves the experience quite a bit!

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #22)
> about the infoPane, the height could still be set up in the localstore.rdf for
> it, it should shrink when nothing is selected to leave some more space, but if
> the height is set up there it does not shrink.

So the user doesn't have control about the height in any way? I'm not able to move the horizontal splitter. Is this expected?

Comment 26

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #25)
> So the user doesn't have control about the height in any way? I'm not able to
> move the horizontal splitter. Is this expected?

Yes, the pane takes height of its contents (apart bug 428020 problem)

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Too tall and not sizable details pane

I end-up in such a sectioning of the bookmarks list and the details pane. There is no way to shrink the size of the lower pane.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 314718 [details]
Too tall and not sizable details pane

Sorry, I hit bug 428020. Removing localstore.rdf solves it for the current profile.

Comment 29

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #28)
> (From update of attachment 314718 [details])
> Sorry, I hit bug 428020. Removing localstore.rdf solves it for the current
> profile.

yes please annotate on that bug and post the same screenshot

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040904 Minefield/3.0pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre ID:2008040907

Comment 31

[Dão Gottwald [:dao]]

(In reply to comment #14)
> Created an attachment (id=314212) [details]
> patch
> 
> Reverted splitter style as requested.
> 
> fixed here:
[..]
> - winstripe left splitter styled like sidebar splitter

This didn't really work. See bug 431639.

Comment 32

[Tracy Walker [:tracy]]

not really an item that's needed to be checked in litmus

Comment 33

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv