1067228 - [Messages][Refactoring] Move subject management to a separate component

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

In the scope of this bug we need to remove direct Thread UI -> Subject DOM dependencies and move subject specific code to separate (web)component. It should simplify solution for bug 1040144.

Comment 1

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Hey Julien,

This patch is almost ready, I just have one issue that I mentioned on GitHub, but it would be really helpful to get your early feedback.

Thanks!

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 8494414 [details] [review]
GitHub pull request URL

mostly good stuff, with some cleanup along the way.

Added comments to github, we can discuss about it on IRC :)

Comment 3

[Oleg Zasypkin [:azasypkin]]

Hey Jenny,

We have question for you :)

When user edits message subject and subject length reaches some max value (currently 40 symbols) we notify user with "banner" about that. This banner is automatically removed after _2s_ or immediately if user removes the subject. But currently we have one more case when this banner is removed immediately - when subject looses focus.

Do you think the last case makes sense even that banner will be removed after 2s anyway? We're asking about that because supporting this case requires some nasty code that we'd like to remove :)

Thanks!

Comment 4

[Jenny Lee]

Hello Oleg,

I think it's ok to not remove the banner immediately when subject field loses focus. Go ahead and remove the code :D

Comment 5

[Oleg Zasypkin [:azasypkin]]

(In reply to Jenny Lee from comment #4)
> Hello Oleg,
> 
> I think it's ok to not remove the banner immediately when subject field
> loses focus. Go ahead and remove the code :D

Great, thanks!

Comment 6

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8494414 [details] [review]
GitHub pull request URL

Hey Julien,

I've updated PR with comments you suggested on IRC. Could you please take a look?

Thanks!

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8494414 [details] [review]
GitHub pull request URL

r=me with the nits we discussed on IRC (no multiline ternary operator, don't use classList.toggle if we have a nearby if-block with the same condition)

thanks for the great work !

Comment 8

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> Comment on attachment 8494414 [details] [review]
> GitHub pull request URL
> 
> r=me with the nits we discussed on IRC (no multiline ternary operator, don't
> use classList.toggle if we have a nearby if-block with the same condition)
> 
> thanks for the great work !

Thanks for review! Nits fixed and Try is green :)

Master: https://github.com/mozilla-b2g/gaia/commit/097d453f77594a752dccfeb07ad05e775fbf7be3