954809 - Followup: Unread ruler CSS and Bubbles margin improvements

Status: RESOLVED FIXED

(Instantbird Graveyard :: Conversation, defect)

Description

[aleth [:aleth]]

*** Original post on bio 1375 at 2012-04-13 12:32:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***

Comment 1

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 2

[Patrick Cloke [:clokep]]

*** 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.

Comment 3

[aleth [:aleth]]

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 ***

Comment 4

[Benedikt Pfeifer]

*** 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;

Comment 5

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 6

[aleth [:aleth]]

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 ***

Comment 7

[Benedikt Pfeifer]

*** 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;

Comment 8

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 9

[aleth [:aleth]]

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 ***

Comment 10

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 11

[aleth [:aleth]]

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 ***

Comment 12

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1375 as attmnt 1350 at 2012-04-13 19:30:00 UTC ***

- Minor color tweak.

Comment 13

[aleth [:aleth]]

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 ***

Comment 14

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 15

[aleth [:aleth]]

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 ***

Comment 16

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1375 as attmnt 1361 at 2012-04-15 14:36:00 UTC ***

- Save another line of CSS.

Comment 17

[aleth [:aleth]]

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 ***

Comment 18

[aleth [:aleth]]

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 ***

Comment 19

[aleth [:aleth]]

Attachment: Patch

*** 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.

Comment 20

[aleth [:aleth]]

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 ***

Comment 21

[aleth [:aleth]]

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.

Comment 22

[aleth [:aleth]]

Attachment: Patch with tighter margins

*** 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.

Comment 23

[aleth [:aleth]]

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 ***

Comment 24

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1375 as attmnt 1588 at 2012-06-11 23:10:00 UTC ***

Add comments and constants as discussed on IRC.

Comment 25

[aleth [:aleth]]

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 ***

Comment 26

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1375 as attmnt 1589 at 2012-06-11 23:18:00 UTC ***

Tweaked comment.

Comment 27

[aleth [:aleth]]

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 ***

Comment 28

[Florian Quèze [: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 ***

Comment 29

[Patrick Cloke [:clokep]]

*** 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!