Closed Bug 954145 Opened 10 years ago Closed 10 years ago

Dark variant of the Simple skin

Categories

(Instantbird Graveyard :: Other, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: bugzilla)

Details

Attachments

(2 files, 2 obsolete files)

*** Original post on bio 710 by Albin Jacobsson [:Sertion] <albin.jacobsson AT gmail.com> at 2011-02-20 20:21:00 UTC ***

It would be nice to have a darker version of the default 'Simple skin'. Nothing fancy, but something to switch to on those late night sessions.
Attached patch Dark variant of the Simple skin (obsolete) — Splinter Review
*** Original post on bio 710 as attmnt 525 by albin.jacobsson AT gmail.com at 2011-02-20 20:46:00 UTC ***

This is a proposed patch. It's not completley dark (background color of #222), but a nice contrast to the default white variant of the skin.
Comment on attachment 8352266 [details] [diff] [review]
Dark variant of the Simple skin

*** Original change on bio 710 attmnt 525 by albin.jacobsson AT gmail.com at 2011-02-20 21:02:05 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352266 - Flags: review?(bugzilla)
Assignee: nobody → bugzilla
Attached image Screenshot
*** Original post on bio 710 as attmnt 527 at 2011-02-21 00:23:00 UTC ***

I applied the patch to see how the proposed variant looks. I guess a screenshot will save idechix (and others reading the bug) some time ;).
*** Original post on bio 710 at 2011-02-21 00:44:47 UTC ***

Just confirming this since it has a patch, etc.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
*** Original post on bio 710 by Albin Jacobsson [:Sertion] <albin.jacobsson AT gmail.com> at 2011-02-21 10:21:59 UTC ***

The currently submitted diff is not complete. Will fix as soon as I can get access to a computer.
*** Original post on bio 710 as attmnt 531 by albin.jacobsson AT gmail.com at 2011-02-21 15:33:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352272 - Flags: review?(bugzilla)
Comment on attachment 8352266 [details] [diff] [review]
Dark variant of the Simple skin

*** Original change on bio 710 attmnt 525 by albin.jacobsson AT gmail.com at 2011-02-21 15:33:45 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352266 - Attachment is obsolete: true
Attachment #8352266 - Flags: review?(bugzilla)
Comment on attachment 8352272 [details] [diff] [review]
Dark variant of the Simple skin (with readable links)

*** Original change on bio 710 attmnt 531 by idechix AT instantbird.org at 2011-02-21 16:28:32 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352272 - Flags: review?(bugzilla) → review+
*** Original post on bio 710 at 2011-02-21 19:47:09 UTC ***

(In reply to comment #5)
> Created an attachment (id=531) [details]
> Dark variant of the Simple skin (with readable links)

Is there a reason why you prefer rgb(...) over hex color codes?

Here's the hex codes:
 rgb(120,120,220) -> #7878DC
 rgb(220,120,120) -> #DC7878

Looks good by the way :)
*** Original post on bio 710 by Albin Jacobsson [:Sertion] <albin.jacobsson AT gmail.com> at 2011-02-21 20:04:38 UTC ***

(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=531) [details] [details]
> > Dark variant of the Simple skin (with readable links)
> 
> Is there a reason why you prefer rgb(...) over hex color codes?
> 
> Here's the hex codes:
>  rgb(120,120,220) -> #7878DC
>  rgb(220,120,120) -> #DC7878
> 
> Looks good by the way :)

I prefer hex, but they are rgb in the main.css-file so I thought there was a reason for it. Doesn't actually matter tho.
*** Original post on bio 710 at 2011-02-21 22:41:45 UTC ***

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Created an attachment (id=531) [details] [details] [details]
> > > Dark variant of the Simple skin (with readable links)
> > 
> > Is there a reason why you prefer rgb(...) over hex color codes?
> > 
> > Here's the hex codes:
> >  rgb(120,120,220) -> #7878DC
> >  rgb(220,120,120) -> #DC7878
> > 
> > Looks good by the way :)
> 
> I prefer hex, but they are rgb in the main.css-file so I thought there was a
> reason for it. Doesn't actually matter tho.

You're right, it doesn't really matter.
Sorry for bikeshedding.
Comment on attachment 8352272 [details] [diff] [review]
Dark variant of the Simple skin (with readable links)

*** Original change on bio 710 attmnt 531 at 2011-02-22 22:25:00 UTC ***

Some tiny coding style nits to improve consistency across the file:

>diff --git a/instantbird/themes/jar.mn b/instantbird/themes/jar.mn
>--- a/instantbird/themes/jar.mn
>+++ b/instantbird/themes/jar.mn
>@@ -425,3 +425,4 @@
> 	skin/classic/instantbird/messages/simple/main.css              (messages/simple/main.css)
> 	skin/classic/instantbird/messages/simple/Status.html           (messages/simple/Status.html)
> 	skin/classic/instantbird/messages/simple/Variants/Normal.css   (messages/simple/Variants/Normal.css)
>+	skin/classic/instantbird/messages/simple/Variants/Dark.css   (messages/simple/Variants/Dark.css)
Please add 2 spaces here to keep the alignment with the "(messages/..." of the line above. It improves readability. 

>diff --git a/instantbird/themes/messages/simple/Variants/Dark.css b/instantbird/themes/messages/simple/Variants/Dark.css
>new file mode 100644
>--- /dev/null
>+++ b/instantbird/themes/messages/simple/Variants/Dark.css
>@@ -0,0 +1,16 @@
>+body{
Add a space before "{" please.

>+  background: #222;
>+  color:#eee;
and after ":" here.

>+}
>+.outgoing .pseudo {
>+  color: rgb(120,120,220);
>+}

Please use the same color format everywhere in the file (I have a slight preference for the rgb() format that I find more readable, but I really don't mind if you prefer using the hex format, just use it everywhere in the file :)).

Looks good to me too otherwise :).
Attachment #8352272 - Flags: review-
*** Original post on bio 710 as attmnt 533 by albin.jacobsson AT gmail.com at 2011-02-22 22:47:00 UTC ***

Hopefully this patch fixes everything. I use hex because it's easier to copy and paste between different services. Thanks to Benedikt P. [:Mic] for the hex-values.
Comment on attachment 8352272 [details] [diff] [review]
Dark variant of the Simple skin (with readable links)

*** Original change on bio 710 attmnt 531 by albin.jacobsson AT gmail.com at 2011-02-22 22:47:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352272 - Attachment is obsolete: true
*** Original post on bio 710 at 2011-02-22 23:14:53 UTC ***

Pushed as https://hg.instantbird.org/instantbird/rev/fb7ccbb54b64

It should be usable in the next nightly. Congrats for your first patch in Instantbird! :)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a2
*** Original post on bio 710 by Albin Jacobsson [:Sertion] <albin.jacobsson AT gmail.com> at 2011-02-22 23:33:54 UTC ***

(In reply to comment #11)
> Pushed as https://hg.instantbird.org/instantbird/rev/fb7ccbb54b64
> 
> It should be usable in the next nightly. Congrats for your first patch in
> Instantbird! :)

Great, thank you. :D
You need to log in before you can comment on or make changes to this bug.