1008095 - [Follow up] [Building Blocks] Split headers.css to demo @import

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Looks like the build system is not yet ready to support @import in the shared directory.
So doing manual inline in this patch

Comment 1

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Attachment: patch in github

Comment 2

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8419976 [details] [review]
patch in github

looks like revert commit for bug 1000132 but commented |@import|, so r=yurenju

please land it after getting green on travis-ci.

BTW, we will have build test for DEBUG=1 on bug 1006950, so we will catch kind of this bug in future.

Comment 3

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Merged: f21cc84fc04204934e866c3a4f5a66bbdaee8e13

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

Unfortunat(In reply to Yuren [:yurenju] from comment #2)
> Comment on attachment 8419976 [details] [review]
> patch in github
> 
> looks like revert commit for bug 1000132 but commented |@import|, so
> r=yurenju

Unfortunately, this was not just a revert commit for bug 100132. If you look at the diff of headers.css before and after bug 1000132 + bug 1008095, there are bunch of changes to the headers rules [1], as well as some added "skins". Arnau, I thought the purpose of bug 1000132 was to demo css imports, so why did the basic header rules change too? Also, if bug 1000132 caused a build breakage, why didn't we just back that out instead of filing a follow up here? If you look at bug 1000132, it would appear we resolved/fixed the ability to demo imports. But in reality we just landed some header css changes unrelated (I think?) to css imports, added some css skin rules, and some commented css import lines. What is the state of imports, and what were the changes that these bugs landed?

The reason I ask is because I'm working on bug 968483, and its pretty hard to keep in sync with what's going on with headers.css atm.


https://github.com/mozilla-b2g/gaia/compare/c3f7b6a1692cc7d94d35002cc1d5aa236653e6d7...a01ac23b333066bf1fc0a32cea5e8a4527150d40#diff-bf4681e05dbcb7bf87891b922998750dL1

Comment 5

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

(In reply to Michael Henretty [:mhenretty] from comment #4)
> Unfortunat(In reply to Yuren [:yurenju] from comment #2)
> > Comment on attachment 8419976 [details] [review]
> > patch in github
> > 
> > looks like revert commit for bug 1000132 but commented |@import|, so
> > r=yurenju
> 
> Unfortunately, this was not just a revert commit for bug 100132. If you look
> at the diff of headers.css before and after bug 1000132 + bug 1008095, there
> are bunch of changes to the headers rules [1], as well as some added
> "skins". Arnau, I thought the purpose of bug 1000132 was to demo css
> imports, so why did the basic header rules change too? Also, if bug 1000132
> caused a build breakage, why didn't we just back that out instead of filing
> a follow up here? If you look at bug 1000132, it would appear we
> resolved/fixed the ability to demo imports. But in reality we just landed
> some header css changes unrelated (I think?) to css imports, added some css
> skin rules, and some commented css import lines. What is the state of
> imports, and what were the changes that these bugs landed?
> 
> The reason I ask is because I'm working on bug 968483, and its pretty hard
> to keep in sync with what's going on with headers.css atm.
> 
> 
> https://github.com/mozilla-b2g/gaia/compare/
> c3f7b6a1692cc7d94d35002cc1d5aa236653e6d7...
> a01ac23b333066bf1fc0a32cea5e8a4527150d40#diff-
> bf4681e05dbcb7bf87891b922998750dL1

Michael, I know this patch could be a mess to solve conflicts for anybody working with header.css right now.

bug 1000132 had two purposes:
-demoing @import in Building blocks, to have a cleaner markup both for skins and larger devices (tablets..)
-adding 'gaia-header' selector to support the new web component we are working on

After landing bug 1000132, Yuren found out DEBUG=1 was not working because @import in shared folder is not yet supported.

We had to options then:
-back the patch out
-try to manually inline the code in header.css and still make the code compatible with 'gaia-header'. So I decided to go for this one, as we are in a rush to start using web components in 2.0

Sorry for the inconveniences, and please let me know if you need any help with headers.css

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Arnau March  [:arnau] from comment #5)
> bug 1000132 had two purposes:
> -demoing @import in Building blocks, to have a cleaner markup both for skins
> and larger devices (tablets..)
> -adding 'gaia-header' selector to support the new web component we are
> working on

Gotcha. It would have been more clear if that work was done in separate bugs. Especially since moving code into a separate file with changes in one commit, and then moving the changed code back in another commit makes it hard to look at the history and see how the code changed.

`git diff c3f7b6 a01ac2 -- shared/style/headers.css` does the trick though :)

I'll work on integrating your changes into Pavel's WIP for back-button, and will probably flag you on Monday to make sure I'm doing it right. Thanks!