Closed
Bug 954541
Opened 10 years ago
Closed 10 years ago
Contact list section header styling for Linux
Categories
(Instantbird Graveyard :: Contacts window, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: aleth, Assigned: aleth)
Details
(Whiteboard: [1.2-wanted])
Attachments
(3 files, 6 obsolete files)
*** Original post on bio 1107 at 2011-10-21 17:44:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1107 as attmnt 918 at 2011-10-21 17:44:00 UTC *** This is a Linux adaptation of Mic's Windows/Aero patch to bug 954413 (bio 979). I experimented with the gradient Mic added to the section headers, but it was hard to find something which fits in across various GTK/KDE themes. I ended up settling for some subtle borders, which looks quite good I think.
Attachment #8352660 -
Flags: review?
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1107 as attmnt 919 at 2011-10-21 17:45:00 UTC *** Some screenshots. Someone should probably check how this looks on the latest ubuntu before accepting the patch!
Comment 3•10 years ago
|
||
Comment on attachment 8352660 [details]
Patch for instantbird/themes/blist.css
*** Original change on bio 1107 attmnt 918 at 2011-10-25 16:04:42 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352660 -
Attachment is patch: false
Attachment #8352660 -
Attachment mime type: text/plain → text/css
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1107 as attmnt 982 at 2011-11-09 01:02:00 UTC *** Now a diff - not sure what went wrong there.
Attachment #8352723 -
Flags: review?(florian)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8352660 [details]
Patch for instantbird/themes/blist.css
*** Original change on bio 1107 attmnt 918 at 2011-11-09 01:02:49 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352660 -
Attachment is obsolete: true
Attachment #8352660 -
Flags: review?
Assignee | ||
Updated•10 years ago
|
Whiteboard: [1.2-wanted]
Comment 6•10 years ago
|
||
Comment on attachment 8352723 [details] [diff] [review] Patch for instantbird/themes/blist.css *** Original change on bio 1107 attmnt 982 at 2012-01-30 11:24:47 UTC *** I'm not sure of why a patch for a Linux-only bug changes the CSS for non-Mac (that is both Linux and Windows), but that doesn't look like what you intended to do.
Attachment #8352723 -
Flags: review?(florian) → review-
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1142 at 2012-01-30 12:36:00 UTC *** Back in October when I did this, I thought XP_MACOSX referred to Windows (XP, i.e. non-Aero) and OSX ;) Shame there is not XP_LINUX, it would make things more legible.
Attachment #8352887 -
Flags: review?(florian)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8352723 [details] [diff] [review] Patch for instantbird/themes/blist.css *** Original change on bio 1107 attmnt 982 at 2012-01-30 12:36:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352723 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
*** Original post on bio 1107 at 2012-01-30 13:18:41 UTC *** Comment on attachment 8352887 [details] [diff] [review] (bio-attmnt 1142) Patch >+%ifndef XP_MACOSX >+%ifndef XP_WIN I suggest you use this instead as it only involves OSs that have XP_UNIX defined: %ifdef XP_UNIX %ifndef XP_MACOSX
Comment 10•10 years ago
|
||
Comment on attachment 8352887 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1142 at 2012-04-24 16:52:29 UTC *** Per Mic's suggestion. For the actual look of it, I think it looks fine (and I think you're nominally in charge of our *NIX theme, so...I'm OK with the changes).
Attachment #8352887 -
Flags: review?(florian) → review-
Assignee | ||
Comment 11•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1390 at 2012-04-24 16:57:00 UTC *** OK
Attachment #8353143 -
Flags: review?(clokep)
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8352887 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1142 at 2012-04-24 16:57:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352887 -
Attachment is obsolete: true
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8353143 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1390 at 2012-04-24 17:03:10 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353143 -
Flags: review?(clokep)
Assignee | ||
Comment 14•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1391 at 2012-04-24 17:06:00 UTC *** Typo fix.
Attachment #8353144 -
Flags: review?(clokep)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8353143 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1390 at 2012-04-24 17:06:59 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353143 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1402 at 2012-04-25 11:38:00 UTC *** Changes now integrated into existing ifdef tree. Not sure why I didn't do that originally.
Attachment #8353155 -
Flags: review?(clokep)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8353144 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1391 at 2012-04-25 11:38:08 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353144 -
Attachment is obsolete: true
Attachment #8353144 -
Flags: review?(clokep)
Comment 18•10 years ago
|
||
*** Original post on bio 1107 at 2012-04-25 11:49:18 UTC *** Comment on attachment 8353155 [details] [diff] [review] (bio-attmnt 1402) Patch >--- blist-old.css 2012-04-25 12:08:09.000000000 +0200 >+++ blist.css 2012-04-25 13:33:02.000000000 +0200 >@@ -68,39 +68,53 @@ > border-bottom-color: rgba(0, 0, 0, 0.32); > %else > background-color: -moz-Dialog; > %ifdef XP_WIN > background-image: -moz-linear-gradient(rgba(255,255,255,.5), rgba(255,255,255,0)); > %else > background-image: -moz-linear-gradient(rgba(255,255,255,.3), rgba(255,255,255,0)); > %endif >+%ifndef XP_UNIX > %ifndef XP_WIN If not XP_UNIX, if not XP_WIN...what's left?! (But seriously, I think you might be able to just remove this ifdef then?) > border-bottom: 1px solid ThreeDShadow; > %else > border-bottom: none; > %endif > %endif >+%endif > } > #contactsHeader { > %ifdef XP_MACOSX > border-top: 1px solid rgb(66, 66, 66); > } > > #contactsHeader:-moz-window-inactive { > border-top-color: rgb(137, 137, 137); > %else >+%ifdef XP_UNIX >+ border-top: 1px solid rgba(0, 0, 0, 0.1); >+%else > %ifndef XP_WIN > border-top: 1px solid ThreeDShadow; > %else > border-top: none; > %endif This seems confusing for me, I know it's not XP_MACOSX, then we have a statement (you added) for XP_UNIX, then we have not XP_WIN and other. So it seems like there's two things being set for UNIX. > %endif >+%endif >+} >+ >+%ifdef XP_UNIX >+%ifndef XP_MACOSX >+#convsHeader { >+ border-top: 1px solid rgba(0, 0, 0, 0.12); > } >+%endif >+%endif A flowchart would be useful. ;) :)
Assignee | ||
Comment 19•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1403 at 2012-04-25 13:16:00 UTC ***
> If not XP_UNIX, if not XP_WIN...what's left?! (But seriously, I think you might
> be able to just remove this ifdef then?)
I wasn't completely sure there weren't any obscure platforms I didn't know about.
So this patch is a little simpler ;)
Attachment #8353156 -
Flags: review?(clokep)
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8353155 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1402 at 2012-04-25 13:16:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353155 -
Attachment is obsolete: true
Attachment #8353155 -
Flags: review?(clokep)
Comment 21•10 years ago
|
||
*** Original post on bio 1107 at 2012-04-25 13:55:23 UTC *** Comment on attachment 8353156 [details] [diff] [review] (bio-attmnt 1403) Patch (In reply to comment #11) > Created attachment 8353156 [details] [diff] [review] (bio-attmnt 1403) [details] > Patch > > If not XP_UNIX, if not XP_WIN...what's left?! (But seriously, I think you might > > be able to just remove this ifdef then?) > I wasn't completely sure there weren't any obscure platforms I didn't know > about. I don't think there's any we care about here, if there is...well we don't distribute them. :) > -%ifndef XP_WIN > - border-bottom: 1px solid ThreeDShadow; > -%else > - border-bottom: none; > -%endif Won't this change XP_WIN also?
Assignee | ||
Comment 22•10 years ago
|
||
*** Original post on bio 1107 at 2012-04-25 14:03:30 UTC *** (In reply to comment #12) > > -%ifndef XP_WIN > > - border-bottom: 1px solid ThreeDShadow; > > -%else > > - border-bottom: none; > > -%endif > > Won't this change XP_WIN also? Yes, but the only other XP_WIN border CSS I can see here is border-style: none, so it seems superfluous.
Comment 23•10 years ago
|
||
Comment on attachment 8353156 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1403 at 2012-04-26 00:50:19 UTC *** This patch looks a lot better. :) I can't really give a UI review since I don't have Linux. :( The screenshots look fine though.
Attachment #8353156 -
Flags: review?(clokep) → review+
Updated•10 years ago
|
Attachment #8353156 -
Flags: review?(florian)
Comment 24•10 years ago
|
||
Comment on attachment 8353156 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1403 at 2012-07-17 15:11:33 UTC *** I now have Linux, so setting this back to me for a UI review.
Attachment #8353156 -
Flags: review?(florian)
Attachment #8353156 -
Flags: review?(clokep)
Attachment #8353156 -
Flags: review+
Comment 25•10 years ago
|
||
*** Original post on bio 1107 as attmnt 1771 at 2012-08-02 00:42:00 UTC *** Here's how this looks under unity with and without the patch applied.
Comment 26•10 years ago
|
||
Comment on attachment 8353532 [details]
Screenshot under unity (left: with patch, right: without patch)
*** Original change on bio 1107 attmnt 1771 at 2012-08-02 00:48:09 UTC ***
Oops, I meant the left one is WITH the patch and the right one is WITHOUT the patch.
Attachment #8353532 -
Attachment description: Screenshot under unity (left: without patch, right: with patch) → Screenshot under unity (left: with patch, right: without patch)
Comment 27•10 years ago
|
||
Comment on attachment 8353156 [details] [diff] [review] Patch *** Original change on bio 1107 attmnt 1403 at 2012-08-02 00:52:19 UTC *** I think this is an improvement on all Linux desktop managers we tested.
Attachment #8353156 -
Flags: review?(clokep) → review+
Updated•10 years ago
|
Whiteboard: [1.2-wanted] → [1.2-wanted][checkin-needed]
Comment 28•10 years ago
|
||
*** Original post on bio 1107 at 2012-08-02 18:10:20 UTC *** Finally checked in as http://hg.instantbird.org/instantbird/rev/cdc65c66b724 Thanks! :) Sorry for the long review.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [1.2-wanted][checkin-needed] → [1.2-wanted]
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•