Modern theme for Lightning, Part 1

NEW
Unassigned

Status

Calendar
Lightning: SeaMonkey Integration
9 years ago
2 years ago

People

(Reporter: Ian Neal, Unassigned)

Tracking

(Blocks: 2 bugs, {modern})

Dependency tree / graph

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

9 years ago
When using SeaMonkey and Lightning all the current theming looks reasonably okay for SM's default theme but not for the modern theme.

Updated

9 years ago
Keywords: calendar-integration, modern

Updated

9 years ago
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED

Comment 1

9 years ago
Created attachment 402611 [details] [diff] [review]
Patch 1.0 Modern theme for Lightning.

First attempt. Asking for first review from Neil as he's the modern expert. Eventually I'll ask for review from a Calendar peer.
Attachment #402611 - Flags: review?(neil)

Comment 2

9 years ago
Comment on attachment 402611 [details] [diff] [review]
Patch 1.0 Modern theme for Lightning.

Working on a new patch.
Attachment #402611 - Flags: review?(neil)

Comment 3

9 years ago
Created attachment 404425 [details] [diff] [review]
Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil

> +% skin lightning modern/1.0 %skin/modern/
This patch adds a modern skin provider for the lightning package. This is relatively simple as there are only four css files. I just copied the lighting winstripe files and replaced all the system colours with hard coded colour values. I just made random guesses as to the colours but I'm probably wrong. I also added several icons and pointed the css at these instead of non-existent classic equivalents in modern.

> +/* The following override rules from chrome://calendar/skin/ etc */
+
The classic theme for the calendar package has significantly more css files and I wasn't up to writing a complete modern skin for /calendar/. So basically I've tacked on some styles at the end of lightning.css to override several styles coming from the classic /calendar/ skin.

> diff --git a/calendar/lightning/themes/modern/icons/collapse.png b/calendar/lightning/themes/modern/icons/collapse.png

> diff --git a/calendar/lightning/themes/modern/icons/expand.png b/calendar/lightning/themes/modern/icons/expand.png
copied from tookit winstripe/global/.

> diff --git a/calendar/lightning/themes/modern/icons/folder-trash.png b/calendar/lightning/themes/modern/icons/folder-trash.png
Copied from Thunderbird /pinstripe/

> diff --git a/calendar/lightning/themes/modern/icons/twisty-clsd.gif b/calendar/lightning/themes/modern/icons/twisty-clsd.gif
> diff --git a/calendar/lightning/themes/modern/icons/twisty-open.gif b/calendar/lightning/themes/modern/icons/twisty-open.gif
Copied from SM messenger modern.

>+.unifinder-closebutton,

>+#today-closer {
>
+  list-style-image: url("chrome://lightning/skin/icons/close.png");
Would it be better to use the close-*.gif graphics in SM modern/global/icons/ ?
Attachment #402611 - Attachment is obsolete: true
Attachment #404425 - Flags: superreview?(neil)
Attachment #404425 - Flags: review?(iann_bugzilla)

Updated

9 years ago
Attachment #404425 - Flags: ui-review?(clarkbw)

Comment 4

9 years ago
Comment on attachment 404425 [details] [diff] [review]
Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil

Brian, I would appreciate your comments on this. Thanks.

Comment 5

9 years ago
Comment on attachment 404425 [details] [diff] [review]
Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil

As random guesses go, #F0ECE4, #0A6CD0 and #BAEEFF look pretty random to me.
There's also a distinct lack of namespace declarations, by the way.
(Reporter)

Comment 6

9 years ago
I know this bug is for modern but I've noticed there is probably a few places where we need to replace classic css rules too (e.g. accountCentral)

Updated

9 years ago
Attachment #404425 - Flags: superreview?(neil) → superreview+
(Reporter)

Updated

9 years ago
Attachment #404425 - Flags: review?(iann_bugzilla) → review-
(Reporter)

Comment 7

9 years ago
Comment on attachment 404425 [details] [diff] [review]
Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil

The Today Pane/Sidebar is still lacking modern theming - today-pane.css
(Reporter)

Comment 8

9 years ago
There is also an issue with the calendar-event-dialog window button toolbar both in its background colour and how hovering over the buttons works.

Comment 9

9 years ago
> (From update of attachment 404425 [details] [diff] [review])
> The Today Pane/Sidebar is still lacking modern theming - today-pane.css

Yes I know. But as I said I'm not up to providing a complete skin for the /calendar/ package as well as /lightning/. The styles tacked to the end of lightning.css are the minimum needed for any *noticeably* missing UI to show up.

> There is also an issue with the calendar-event-dialog window button toolbar
> both in its background colour and how hovering over the buttons works.

Again, that's from the /calendar/ package and doesn't use any style files from /lightning/. If you want a modern skin provider for /calendar/ as well, somebody else will have to do it.
(Reporter)

Comment 10

9 years ago
Comment on attachment 404425 [details] [diff] [review]
Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil

okay r=me, but could you spin off a bug for the changes needed in /calendar/ to give lightning a modern theme and mark the dependencies?
Attachment #404425 - Flags: review- → review+

Updated

9 years ago
Summary: Need modern theme for Lightning → Modern theme for Lightning, Part 1

Comment 11

9 years ago
Moving to the Calendar product.
Component: Themes → Lightning: SeaMonkey Integration
Product: SeaMonkey → Calendar
QA Contact: themes → lightning-seamonkey
Target Milestone: --- → 1.0

Updated

9 years ago
Blocks: 521228

Comment 12

9 years ago
> Moving to the Calendar product.
Lost the sr+ from Neil in the move.

> but could you spin off a bug for the changes needed in /calendar/ to
> give lightning a modern theme and mark the dependencies?
Filed Bug 521228.

Comment 13

9 years ago
Created attachment 405258 [details] [diff] [review]
Patch v1.2 with added namespace to the CSS files.

This patch has r=IanN and sr=neil from SeaMonkey reviewers. I've moved this bug to Calendar/Lightning and I'm asking for review from a Calendar peer.
Attachment #404425 - Attachment is obsolete: true
Attachment #405258 - Flags: review?(clarkbw)
Attachment #404425 - Flags: ui-review?(clarkbw)

Updated

9 years ago
Attachment #404425 - Attachment description: Patch 1.1 Updated Modern skin for Lightning. → Patch 1.1 Updated Modern skin for Lightning. r=IanN sr=Neil
Attachment #405258 - Flags: review?(clarkbw) → review?(philipp)
Comment on attachment 405258 [details] [diff] [review]
Patch v1.2 with added namespace to the CSS files.

I think you want Phillipp
Comment on attachment 405258 [details] [diff] [review]
Patch v1.2 with added namespace to the CSS files.

I can do a ui-review of this if that is what you wanted as well
What will this cost us in terms of package size? Personally I'm reluctant to take this, as it increases the theme maintenance burden for Lightning developers significantly. 

My baseline is that we've been working with our two themes (Winstripe and Pinstripe) for quite some time, even though Thunderbird used Qute on Windows, our theme fit in just fine as it does for Classic on SeaMonkey.

As application-specific implementation details (like the decision of SeaMonkey to ship two themes) should not affect extensions like Lightning, I'd propose that you offer the Modern theme for Lightning as an add-on on AMO.

Therefore I'm proposing to WONTFIX this bug. In the end it's Philipp's call, though.
Keywords: calendar-integration
I wouldn't want to destroy the work here, although I agree that we'd like to keep the number of themes down. One thing that has been annoying me in lightning is that we have a lot of theming that is common between pinstripe/ and winstripe. In an ideal world, we could keep anything that is true for all themes in our common/ folder and just do theme specific tweaks in pinstripe, winstripe(,gnomestripe, qute, modern, whatever). If this were the case, then the maintenance is not as significant anymore and we might be able to live with additional themes.

What do you think about putting this theme on AMO? Is this an option for you?

Comment 18

9 years ago
> What do you think about putting this theme on AMO? Is this an option for you?
I'll see what KaiRo says.

Comment 19

9 years ago
Created attachment 406016 [details]
Lightning Mods extension for SeaMonkey 0.1pre

This is an XPI. Additions since the last patch:

1. Packaged as an XPI.
2. Moved the /calendar/ overrides to chrome://lightningmods/skin/messengerOverlay.css
3. In Classic tweak the msgHeaderView buttons and give the delete button an image that exists in our messenger skin.
Attachment #405258 - Attachment is obsolete: true
Attachment #406016 - Flags: review?(iann_bugzilla)
Attachment #405258 - Flags: review?(philipp)
(Reporter)

Comment 20

9 years ago
(In reply to comment #19)
> Created an attachment (id=406016) [details]
> Lightning Mods extension for SeaMonkey 0.1pre
> 
> This is an XPI. Additions since the last patch:
> 
> 1. Packaged as an XPI.
> 2. Moved the /calendar/ overrides to
> chrome://lightningmods/skin/messengerOverlay.css
> 3. In Classic tweak the msgHeaderView buttons and give the delete button an
> image that exists in our messenger skin.

So far so good, other things I've spotted are:
1. In both Classic and Modern the Today Pane has a grippy next to the New Event button which probably shouldn't be there.
2. In Modern the Account Central "Create a new calendar" icon is missing.
3. In Modern the Today Pane needs the correct background colour.

Comment 21

9 years ago
Created attachment 406629 [details]
Lightning Mods extension [update 1]

> 1. In both Classic and Modern the Today Pane has a grippy next to the New Event
> button which probably shouldn't be there.
Fixed.

> 2. In Modern the Account Central "Create a new calendar" icon is missing.
Aaargh, regression. Fixed.

> 3. In Modern the Today Pane needs the correct background colour.
Fixed.

Also in Classic->Tasks tab the Task Completed button is a menu-button unlike the other message header buttons. On Windows this makes it a double button with one inside the other and both with borders. I've tried a different approach this time. I've removed the borders of the inner button except for the right border so that there is a visual separation from the drop marker. I have no idea how this looks like on Linux and Mac however.

I've also uploaded this XPI to mozdev and you can get the latest version from:
<http://downloads.mozdev.org/xsidebar/mods/lightningmods-dev.xpi>
Attachment #406016 - Attachment is obsolete: true
Attachment #406629 - Flags: review?(iann_bugzilla)
Attachment #406016 - Flags: review?(iann_bugzilla)
(Reporter)

Updated

8 years ago
Attachment #406629 - Flags: review?(iann_bugzilla)

Updated

8 years ago
Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
Target Milestone: 1.0 → ---

Comment 22

2 years ago
Now that bug 516026 has landed and Lightning is bundled and enabled by default with SeaMonkey, it would be nice to see some conclusion here. Maybe the theme XPI can just be shipped with SeaMonkey itself if it's not supposed to come with the regular Lightning installer, if that's the blocking issue?

Comment 23

2 years ago
(In reply to rsx11m from comment #22)
> Now that bug 516026 has landed and Lightning is bundled and enabled by
> default with SeaMonkey, it would be nice to see some conclusion here. Maybe
> the theme XPI can just be shipped with SeaMonkey itself if it's not supposed
> to come with the regular Lightning installer, if that's the blocking issue?
The blocking issue is creating Modern style icons for the Modern Theme.
You need to log in before you can comment on or make changes to this bug.