1161985 - [Messages][NG] Split recipient and non-compose related styling from compose.css

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Steve Chung [:steveck]]

Split the non-compose related css into recipient.css for recipient panel/pill styling and compose_view.css for message container.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] steveck-chung:new-message-recipient-css > mozilla-b2g:master

Comment 2

[Steve Chung [:steveck]]

Comment on attachment 8604593 [details] [review]
[gaia] steveck-chung:new-message-recipient-css > mozilla-b2g:master

Hi Oleg, it's a patch that split the compose.css into 3 different parts:
- compose view
- recipient related
- compose module

The goal is to make the styling in composer could be shared between compose/conversation view and compose_view.css/recipients.css are simply for compose panel.

Comment 3

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8604593 [details] [review]
[gaia] steveck-chung:new-message-recipient-css > mozilla-b2g:master

Looks good! Just few tiny suggestions/questions at GitHub.

Thanks!

Comment 4

[Steve Chung [:steveck]]

About the naming for the view and widget, Jenny proposed new_message_view and original composer widget for classification. It sounds fine for me but maybe you have better idea?

Comment 5

[Julien Wajsberg [:julienw]]

Sounds good to me as I already refer to this panel as the "new message panel".

Comment 6

[Oleg Zasypkin [:azasypkin]]

Sounds good to me as well.

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8604593 [details] [review]
[gaia] steveck-chung:new-message-recipient-css > mozilla-b2g:master

Hi Oleg, I updated the patch with your suggestion and new message folder. I tried to rename(git mv) the composer.css to keep the history but git always treat it as add/removal :/

Comment 8

[Julien Wajsberg [:julienw]]

"git mv" is really an alias to add/remove.

However if you use "git diff -M -C" it will find renames (first argument is for (M)ove) and copies (second argument).

Comment 9

[Oleg Zasypkin [:azasypkin]]

I'm afraid we also modified moved file "a bit" so that Git can't detect rename/move. Maybe we'll just do move in a separate PR to preserve the history for composer.css (not sure if we can have 2-commits PR)?

Comment 10

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8604593 [details] [review]
[gaia] steveck-chung:new-message-recipient-css > mozilla-b2g:master

Looks good to me, just two small nits at GitHub.

I don't have any preference about composer.css history loss, maybe it'd better to do the move in a separate PR. What do you think?

Also I've restarted Raptor (se) test at Treeherder that complained about performance regression, don't see how it maybe related to this PR and not sure how to interpret these results (in raptor logs I see only data for Settings app).

Comment 11

[Julien Wajsberg [:julienw]]

You can give arguments to -M too but maybe it's just too much here :)

Comment 12

[Steve Chung [:steveck]]

Maybe the modified part is too much in this case. git can not detect rename even when I set -M50% here, The rename detection will work when I set to -M30% and the renamed file is composer.css to recipients.css here :p

I'm not sure if it worth the keep the history about the styling. Maybe it will be refreshed again for next release, so I'm fine to keep this if you feel it's not that critical.

Comment 13

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/a44b0e188a55c405adf6d499f910f0b791235aa9