Closed Bug 954809 Opened 11 years ago Closed 11 years ago

Followup: Unread ruler CSS and Bubbles margin improvements

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 10 obsolete files)

*** Original post on bio 1375 at 2012-04-13 12:32:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1346 at 2012-04-13 12:32:00 UTC *** - Replace linear background gradient with border. - Tiny margin adjustment as the ruler appears slightly off-center due to the shadow of the preceding bubble.
Attachment #8353099 - Flags: review?(benediktp)
*** Original post on bio 1375 at 2012-04-13 12:59:05 UTC *** Assigning to aleth, who seems to already have a patch for this.
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Hardware: x86 → All
Comment on attachment 8353099 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1346 at 2012-04-13 13:05:42 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353099 - Flags: review?(benediktp)
*** Original post on bio 1375 at 2012-04-13 13:17:49 UTC *** Comment on attachment 8353099 [details] [diff] [review] (bio-attmnt 1346) Patch >diff --git a/chrome/chat/skin/classic/chat/conv.css b/chrome/chat/skin/classic/chat/conv.css > > #unread-ruler { > margin: 0; > width: 100%; >- border: solid; >+ border: dashed Red; > border-width: 1px 0 0 0; >- border-style: dashed; >- border-color: Red; > } I'd rather try border: none; border-top: 1px dashed red; >diff --git a/chrome/instantbird/skin/classic/instantbird/messages/bubbles/main.css b/chrome/instantbird/skin/classic/instantbird/messages/bubbles/main.css > #unread-ruler { >- margin: 1em 0 !important; >+ margin: 13px 0 12px 0 !important; Fine with me if it looks better. > height: 2px; >- border: none !important; >- background-image: -moz-linear-gradient(center top, rgb(211,211,211) 50%, rgb(255,255,255) 50%); >+ border-style: solid none !important; >+ border-width: 1px !important; >+ border-top-color: rgb(211,211,211) !important; >+ border-bottom-color: rgb(255,255,255) !important; > } If something like the following would work, I'd prefer it for clarity: border: 1px solid !important; border-color: rgb(211,211,211) none rgb(255,255,255) !important;
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1347 at 2012-04-13 16:16:00 UTC *** - Implement Mic's suggestions as far as possible. - Set the ruler margins from Bubbles JS so they are always constant. This means the margins of the following bubble remain intact when the ruler is removed. - Set the ruler margin to a better-looking value when the ruler is followed by a time gap text. Bouncing this straight to flo for review as there are no new CSS changes.
Attachment #8353100 - Flags: review?(florian)
Comment on attachment 8353099 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1346 at 2012-04-13 16:16:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353099 - Attachment is obsolete: true
Summary: Followup: Unread ruler CSS improvements → Followup: Unread ruler CSS and Bubbles margin improvements
*** Original post on bio 1375 at 2012-04-13 16:34:34 UTC *** Comment on attachment 8353100 [details] [diff] [review] (bio-attmnt 1347) Patch > #unread-ruler { >- margin: 1em 0 !important; > height: 2px; >- border: none !important; >- background-image: -moz-linear-gradient(center top, rgb(211,211,211) 50%, rgb(255,255,255) 50%); >+ border-style: solid none !important; >+ border-width: 1px !important; >+ border-color: rgb(211,211,211) 0 rgb(255,255,255) !important; Sorry that the first suggestion didn't work. Then I'd say keep the "border: none !important" and add the following for the two borders. Seems clearer, imo: border-top: 1px solid rgb(211,211,211) !important; border-bottom: 1px solid rgb(255,255,255) !important;
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1348 at 2012-04-13 17:39:00 UTC *** - Use Mic's previous comment. - Remove unnecessary height attribute.
Attachment #8353101 - Flags: review?(florian)
Comment on attachment 8353100 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1347 at 2012-04-13 17:39:05 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353100 - Attachment is obsolete: true
Attachment #8353100 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1349 at 2012-04-13 18:57:00 UTC *** - Use alpha rather than a hardcoded rgb color for the dark part of the ruler, in case -moz-dialog etc have unusual values on the user's system.
Attachment #8353102 - Flags: review?(florian)
Comment on attachment 8353101 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1348 at 2012-04-13 18:57:12 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353101 - Attachment is obsolete: true
Attachment #8353101 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1350 at 2012-04-13 19:30:00 UTC *** - Minor color tweak.
Attachment #8353103 - Flags: review?(florian)
Comment on attachment 8353102 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1349 at 2012-04-13 19:30:23 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353102 - Attachment is obsolete: true
Attachment #8353102 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1353 at 2012-04-13 22:06:00 UTC *** - Another 1px tweak for the time text margin. Seems better.
Attachment #8353106 - Flags: review?(florian)
Comment on attachment 8353103 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1350 at 2012-04-13 22:06:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353103 - Attachment is obsolete: true
Attachment #8353103 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1361 at 2012-04-15 14:36:00 UTC *** - Save another line of CSS.
Attachment #8353114 - Flags: review?(florian)
Comment on attachment 8353106 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1353 at 2012-04-15 14:36:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353106 - Attachment is obsolete: true
Attachment #8353106 - Flags: review?(florian)
Comment on attachment 8353114 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1361 at 2012-05-13 23:31:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353114 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1468 at 2012-05-14 09:01:00 UTC *** - Fixes edge case after an account disconnects and then reconnects, where the ruler margins weren't set.
Attachment #8353221 - Flags: review?(florian)
Comment on attachment 8353114 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1361 at 2012-05-14 09:01:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353114 - Attachment is obsolete: true
Comment on attachment 8353221 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1468 at 2012-05-18 15:31:38 UTC *** Code is OK, but I want to experiment with something later.
Attachment #8353221 - Flags: review?(florian)
Attached patch Patch with tighter margins (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1495 at 2012-05-20 23:05:00 UTC *** - 4px less margin. Less margin than this looks too "tight" so that it becomes too hard to quickly spot the unread ruler. I think this version looks better than the current minimum margin. If the reviewer doesn't agree, the previous patch should be taken.
Attachment #8353248 - Flags: review?(florian)
Comment on attachment 8353221 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1468 at 2012-05-20 23:05:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353221 - Attachment is obsolete: true
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1375 as attmnt 1588 at 2012-06-11 23:10:00 UTC *** Add comments and constants as discussed on IRC.
Attachment #8353345 - Flags: review?(florian)
Comment on attachment 8353248 [details] [diff] [review] Patch with tighter margins *** Original change on bio 1375 attmnt 1495 at 2012-06-11 23:10:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353248 - Attachment is obsolete: true
Attachment #8353248 - Flags: review?(florian)
Attached patch PatchSplinter Review
*** Original post on bio 1375 as attmnt 1589 at 2012-06-11 23:18:00 UTC *** Tweaked comment.
Attachment #8353346 - Flags: review?(florian)
Comment on attachment 8353345 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1588 at 2012-06-11 23:18:26 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353345 - Attachment is obsolete: true
Attachment #8353345 - Flags: review?(florian)
Comment on attachment 8353346 [details] [diff] [review] Patch *** Original change on bio 1375 attmnt 1589 at 2012-06-11 23:22:26 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353346 - Flags: review?(florian) → review+
*** Original post on bio 1375 at 2012-06-12 00:26:40 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/b8d8b6e60aef Thanks for this improvement!
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: