992994 - [VDR][Email] Apply Buttons and Input areas [BB]

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Comment 1

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Comment 2

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Comment 3

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

oops ... Sorry Ben

Comment 4

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

James,
Could you please check this patch? I think there's something wrong.
We need to link to buttons and input areas BB in style unstable folder.
For input areas, there's no problem, but buttons looks like it's not being copied inside email app.
So buttons.css is not imported in mail.css

Thanks!

Comment 5

[Marcus Cavanaugh [:mcav]]

Chiming in after IRC discussion with Arnau: It appears the style_unstable files work properly in Nightly but do not seem to get loaded on-device after make reset/install-gaia. I have not done any further investigation.

Comment 6

[James Burke [:jrburke]]

mcav's comment triggered something for me. I think the following line needs to be added to the apps/email/Makefile, after cp line for shared/style:

    cp -rp ../../shared/style_unstable $(STAGE_APP_DIR)/shared

Comment 7

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

\o/ that's it James!
Will add this line to that PR.

Comment 8

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Arnau looks that this works ... :) can you check :) I updated the PR :)

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Note that this patch touches .scrollregion-below-header but seems to be doing what :jrburke is proposing as part of his review feedback for bug 993215 to fix toolbar overlap.  (change it to subtract off 5rem).

Comment 10

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Andrew :) I made the changes

Comment 11

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

(In reply to James Burke [:jrburke] from comment #6)
> mcav's comment triggered something for me. I think the following line needs
> to be added to the apps/email/Makefile, after cp line for shared/style:
> 
>     cp -rp ../../shared/style_unstable $(STAGE_APP_DIR)/shared

James, your suggestion worked great, but looks like Makefile has been update.
Could you please update the file to make it work with style_unstable folder, or let us what should we include in that PR?
Thanks!

Comment 12

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

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

LGTM, just need to fix the makefile :)

Comment 13

[James Burke [:jrburke]]

As this patch is the first new one I know of that wants unstable styles now, I would put the Makefile change in this pull request.

Also, I am sure this was already planned, but be sure to rebase over latest master and remove the style in  apps/email/style/common.css as that style is now already in message_cards.css.

Comment 14

[James Burke [:jrburke]]

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

Can you rebase on top of current master? The build system and the email app have changed noticeably since this patch branch, and I am seeing some weird behavior when trying to apply it, the unstable stuff is not coming over. Once the rebase is done, flip the review back to me.

Comment 15

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

Hey sorry ... I miss to push ... now I think is ok

Comment 16

[James Burke [:jrburke]]

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

So something is still not correct for builds, but it looks like the changes in the email Makefile as part of bug 968661 may be the source of the issue. 

I am still sorting that out, but also trying to sort out a more serious data display issue for another bug. Hope to get this sorted in the next day or two.

Comment 17

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks James :) if I can help with something just ping me :)

Comment 18

[James Burke [:jrburke]]

Comment on attachment 8402756 [details] [review]
patch for Gaia/master

I dug in a little more, and looks like the build system did change enough so to get this patch to work, you'll need to add link tags in the email/index.html for the new css files mentioned in the @import calls. Redundant, but unfortunately necessary for now. 

Locally, I added these two lines to the email/index.html commented-out CSS section and that seemed to fix it:

<link rel="stylesheet" type="text/css" href="shared/style_unstable/input_areas.css" >
<link rel="stylesheet" type="text/css" href="shared/style_unstable/buttons.css" >

If you include that change in the pull request, r+ from me.

Comment 19

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

I just updated the PR but want to be sure before merge ... can you take a look at:
https://github.com/mozilla-b2g/gaia/pull/18045/files#diff-6141e0195f9eca04c4c1480275d69945L14

Comment 20

[James Burke [:jrburke]]

Just pulled the latest from the pull request and pushed to device, did the same reset of the build as before, and still looks good, thanks!

Comment 21

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Landed to master:
https://github.com/mozilla-b2g/gaia/commit/de1c755411949b50ae395b42e124af215ed9b702