Closed
Bug 403140
Opened 17 years ago
Closed 17 years ago
Splitters in the bookmarks organizer should not be collapsible / auto size height of details pane
Categories
(Firefox :: Bookmarks & History, defect, P2)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3
People
(Reporter: faaborg, Assigned: mak)
References
Details
(Keywords: polish)
Attachments
(4 files, 6 obsolete files)
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.
Reporter | ||
Comment 1•17 years ago
|
||
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
Flags: blocking-firefox3?
Comment 2•17 years ago
|
||
These 1990s era UI elements has gots to go.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Assignee | ||
Comment 3•17 years ago
|
||
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)
Assignee | ||
Comment 4•17 years ago
|
||
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
Updated•17 years ago
|
Assignee: nobody → mak77
Reporter | ||
Comment 5•17 years ago
|
||
>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).
Assignee | ||
Comment 6•17 years ago
|
||
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
Attachment #310975 -
Attachment is obsolete: true
Attachment #312047 -
Flags: review?(dietrich)
Assignee | ||
Comment 7•17 years ago
|
||
appearance on Vista after removal of grippy and fixed back-forward
Attachment #312048 -
Flags: ui-review?(beltzner)
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•17 years ago
|
||
Comment on attachment 312047 [details] [diff] [review]
patch
i can probably fix the datails pane to adapt height based on contents
Attachment #312047 -
Flags: review?(dietrich)
Updated•17 years ago
|
Whiteboard: [has patch][needs ui-r beltzner]
Assignee | ||
Comment 9•17 years ago
|
||
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?
Reporter | ||
Comment 10•17 years ago
|
||
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.
Assignee | ||
Updated•17 years ago
|
Attachment #312048 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 11•17 years ago
|
||
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
Attachment #312047 -
Attachment is obsolete: true
Attachment #312236 -
Flags: ui-review?(faaborg)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [has patch][needs ui-r beltzner] → [has patch][needs ui-r faaborg]
Reporter | ||
Comment 12•17 years ago
|
||
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.
Attachment #312236 -
Flags: ui-review?(faaborg) → ui-review?(beltzner)
Updated•17 years ago
|
Whiteboard: [has patch][needs ui-r faaborg] → [has patch][needs ui-r beltzner]
Comment 13•17 years ago
|
||
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
Attachment #312236 -
Flags: ui-review?(beltzner) → ui-review+
Updated•17 years ago
|
Whiteboard: [has patch][needs ui-r beltzner] → [has patch][needs review ?]
Assignee | ||
Comment 14•17 years ago
|
||
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)
Attachment #312236 -
Attachment is obsolete: true
Attachment #314212 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [has patch][needs review ?] → [has patch][needs review mano]
Assignee | ||
Comment 15•17 years ago
|
||
Comment 16•17 years ago
|
||
Looks like the wrong file.
Assignee | ||
Comment 17•17 years ago
|
||
:\ the correct one
Attachment #314212 -
Attachment is obsolete: true
Attachment #314286 -
Flags: review?(mano)
Attachment #314212 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Attachment #314286 -
Attachment is obsolete: true
Attachment #314286 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Attachment #314286 -
Attachment is obsolete: false
Attachment #314286 -
Flags: review?(mano)
Comment 18•17 years ago
|
||
I think we could just remove the scrollbox and don't set any heights.
Assignee | ||
Comment 19•17 years ago
|
||
scrollbox gone
Attachment #314286 -
Attachment is obsolete: true
Attachment #314448 -
Flags: review?(mano)
Attachment #314286 -
Flags: review?(mano)
Comment 20•17 years ago
|
||
Comment on attachment 314448 [details] [diff] [review]
patch
r=mano
Attachment #314448 -
Flags: review?(mano) → review+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Whiteboard: [has patch][needs review mano] → [has patch][has review]
Comment 21•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Assignee | ||
Comment 22•17 years ago
|
||
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?
Assignee | ||
Comment 23•17 years ago
|
||
filled bug 428020 on that
Comment 24•17 years ago
|
||
(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•17 years ago
|
||
(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?
Hardware: PC → All
Whiteboard: [has patch][has review]
Assignee | ||
Comment 26•17 years ago
|
||
(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•17 years ago
|
||
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•17 years ago
|
||
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.
Attachment #314718 -
Attachment is obsolete: true
Assignee | ||
Comment 29•17 years ago
|
||
(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•17 years ago
|
||
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
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Summary: Splitters in the bookmarks organizer should not be collapsable → Splitters in the bookmarks organizer should not be collapsible / auto size height of details pane
Comment 31•17 years ago
|
||
(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•17 years ago
|
||
not really an item that's needed to be checked in litmus
Flags: in-litmus? → in-litmus-
Comment 33•15 years ago
|
||
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•