Open Bug 517684 Opened 11 years ago Updated 4 years ago

Modern theme for Lightning, Part 1

Categories

(Calendar :: Lightning: SeaMonkey Integration, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: iann_bugzilla, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: modern)

Attachments

(1 file, 4 obsolete files)

When using SeaMonkey and Lightning all the current theming looks reasonably okay for SM's default theme but not for the modern theme.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
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 on attachment 402611 [details] [diff] [review]
Patch 1.0 Modern theme for Lightning.

Working on a new patch.
Attachment #402611 - Flags: review?(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)
Attachment #404425 - Flags: ui-review?(clarkbw)
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 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.
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)
Attachment #404425 - Flags: superreview?(neil) → superreview+
Attachment #404425 - Flags: review?(iann_bugzilla) → review-
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
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.
> (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.
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+
Summary: Need modern theme for Lightning → Modern theme for Lightning, Part 1
Moving to the Calendar product.
Component: Themes → Lightning: SeaMonkey Integration
Product: SeaMonkey → Calendar
QA Contact: themes → lightning-seamonkey
Target Milestone: --- → 1.0
Blocks: 521228
> 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.
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)
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.
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?
> What do you think about putting this theme on AMO? Is this an option for you?
I'll see what KaiRo says.
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)
(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.
> 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)
Attachment #406629 - Flags: review?(iann_bugzilla)
Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
Target Milestone: 1.0 → ---
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?
(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.