Closed Bug 1161985 Opened 9 years ago Closed 9 years ago

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


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

Gonk (Firefox OS)
Not set


(Not tracked)



(Reporter: steveck, Assigned: steveck)



(Whiteboard: [sms-sprint-2.2S13])


(1 file)

Split the non-compose related css into recipient.css for recipient panel/pill styling and compose_view.css for message container.
Assignee: nobody → schung
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.
Attachment #8604593 - Flags: feedback?(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.

Attachment #8604593 - Flags: feedback?(azasypkin) → feedback+
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?
Flags: needinfo?(felash)
Flags: needinfo?(azasypkin)
Sounds good to me as I already refer to this panel as the "new message panel".
Flags: needinfo?(felash)
Sounds good to me as well.
Flags: needinfo?(azasypkin)
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 :/
Attachment #8604593 - Flags: review?(azasypkin)
"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).
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 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).
Attachment #8604593 - Flags: review?(azasypkin) → review+
You can give arguments to -M too but maybe it's just too much here :)
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.
Keywords: checkin-needed
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [sms-sprint-2.2S13]
No longer blocks: messages-nga
You need to log in before you can comment on or make changes to this bug.