Closed Bug 431195 Opened 17 years ago Closed 17 years ago

Tree children in Sidebar and Library are indented too far.

Categories

(Toolkit :: XUL Widgets, defect)

x86
Windows Vista
defect
Not set
trivial

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: destian, Assigned: ehsan.akhgari)

References

Details

Attachments

(3 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042806 Firefox/3.0pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042806 Firefox/3.0pre Now that the widgets have been changed over to the Vista style the indentation should probably be reduced to match. The proper indentation is 11px. The following placed in the userchrome will display the proper indentation: treechildren::-moz-tree-indentation { width: 11px !important; } Reproducible: Always Steps to Reproduce: 1. Open Firefox Sidebar or Library 2. Open Windows Explorer in Vista 3. Compare
Could you post before and after screenshots to illustrate what you mean?
Attached image Before/After Indentation (deleted) —
Here is the before and after image.
There was something bugging me about our tree views but I couldn't put my finger on it, thanks for catching this.
Ah, midair collision! Well, now that we've killed the tree lines (*sniffles*), I guess it makes sense to shrink the margins here to make it consistent with the native Aero style. This could probably be rolled up with bug 430895 and bug 431057 as a general Aero tree bug.
Blocks: 425582
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Destian, you really have very good eyes on this :) Few more notes here: 1. The indentation (11px) is too small. On Vista, the tree arrow is directly under the left edge of the parent folder (in both closed and open state). Not too far inside. 2. The spacing between the tree arrow and the folder icon needs to be reduced, instead.
Alex's attachment 311729 [details] is a good reference point.
Component: Places → Theme
QA Contact: places → theme
Component: Theme → XUL Widgets
Product: Firefox → Toolkit
QA Contact: theme → xul.widgets
Glad to help. @Lim Chee Aun 11 pixels will give the correct indentation between the parent's favicon and the child's favicon, but I didn't take into account the distance between the widget and the icon. If the padding-left (margin-left?) is reduced wouldn't the moz-tree-indentation remain at 11px for the correct look? A couple other issues concerning the tree view. The "Closed" widget appears to be slightly smaller than the Vista version and also have a slightly gray center (rather than white/transparent). This may be do to antialiasing. Not sure if this was intended or not and really isn't very important or noticeable. Also, the current Firefox hover effect is to underline while in the vista tree there is a faint blue menulist style hover effect. If there is any interest in fixing either of these I can go ahead and file a bug (or anyone else can feel free to), assuming there isn't one already.
>The "Closed" widget appears to >be slightly smaller than the Vista version and also have a slightly gray center >(rather than white/transparent). This may be do to antialiasing. I just got new images from our designers to fix that. The updated images will land in the next drop. >Also, the current Firefox hover effect is to underline while in the vista tree >there is a faint blue menulist style hover effect. I think we can't get the menulist hover effect working yet for some reason, but we should file a follow up bug if there isn't one yet.
Attached patch Patch (v1) (vista) (deleted) — Splinter Review
This is a pixel-perfect simulation of the positioning of the tree in Vista. Requesting review from Gavin. I'll attach an screenshot comparing my patch to the Vista native tree. Kai, can you handle the XP part?
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #319197 - Flags: review?(gavin.sharp)
Attached image Screenshot of Patch v1 (deleted) —
Sorry for the crude GIMPing... :-)
(In reply to comment #9) > Kai, can you handle the XP part? > I think this is a Vista-only bug; the old "wide" indentation is correct for XP (see attachment 311729 [details])
(In reply to comment #11) > I think this is a Vista-only bug; the old "wide" indentation is correct for XP > (see attachment 311729 [details]) Yeah, you're right. Someone's got to *make* me sleep before I generate more bugspam :/
Comment on attachment 319197 [details] [diff] [review] Patch (v1) (vista) >Index: toolkit/themes/winstripe/global/tree-aero.css >+treechildren::-moz-tree-twisty { >+ padding-right: 1px; > tree[chromedir="rtl"] > treechildren::-moz-tree-twisty { >+ padding-left: 1px; These (and any others like them) should be combined into a single -moz-padding-end rule - can you file a followup?
Attachment #319197 - Flags: review?(gavin.sharp) → review+
Comment on attachment 319197 [details] [diff] [review] Patch (v1) (vista) This is a CSS-only patch to improve the look of trees on Vista. Requesting approval.
Attachment #319197 - Flags: approval1.9?
(In reply to comment #13) > These (and any others like them) should be combined into a single > -moz-padding-end rule - can you file a followup? Sure. I'll post the bug number here.
Depends on: 432272
(In reply to comment #15) > Sure. I'll post the bug number here. -> bug 432272.
Comment on attachment 319197 [details] [diff] [review] Patch (v1) (vista) a1.9=beltzner
Attachment #319197 - Flags: approval1.9? → approval1.9+
mozilla/toolkit/themes/winstripe/global/tree-aero.css 1.2
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: