1072856 - [Messages] Tapping on empty space above send and attach buttons should assimilate currently edited recipient

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

Looks like it worked before, but regressed at some point.

Comment 1

[Mike Hoye [:mhoye]]

Oleg, would you be willing to mentor a new contributor through this?

Comment 2

[Oleg Zasypkin [:azasypkin]]

(In reply to Mike Hoye [:mhoye] from comment #1)
> Oleg, would you be willing to mentor a new contributor through this?
Yeah, it shouldn't be hard to fix. But there is a chance that we'll fix it before we find a volunteer :)

Comment 3

[kumar rishav (:rishav_)]

Attachment: apping on empty space above send and attach buttons should assimilate currently edited recipient.

Hi OLeg
Here is the PR . Please have a look into this.
Thanks

Comment 4

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8501811 [details] [review]
apping on empty space above send and attach buttons should assimilate currently edited recipient.

Hey,

Thanks for the patch! In general it looks good, I just think that we need to move that code to Compose.js instead + few nits on GitHub.

Once you're ready please ask review from Julien or Steve as I won't be around for the next two weeks :)

Comment 5

[kumar rishav (:rishav_)]

Attachment: Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

Hi Juliew
Here is attached PR.
Thanks

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8504834 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

The current patch is fine, but there is one more case that doesn't work like it used to in previous Firefox OS version: we should assimilate when tapping the empty space in the middle of the screen (the space that is between the recipient panel and the composer input).

Can you please have a look to fix this as well? It will probably be in ThreadUI this time.

Thanks a lot !

Comment 7

[kumar rishav (:rishav_)]

Comment on attachment 8504834 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

Hi Julien
I have the updated the patch. Have a look.
Thanks

Comment 8

[Julien Wajsberg [:julienw]]

Comment on attachment 8504834 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

As I said, the code that manages the click on the container needs to be in ThreadUI, because ThreadUI is the component that logically contains this element.

Please request review once you're ready, thanks!

Comment 9

[kumar rishav (:rishav_)]

Comment on attachment 8504834 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

Hi Julien
Hope this time it will be ok. :)
Here is the updated PR .
Thanks

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8504834 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient.

r=me
thanks !

Comment 11

[Julien Wajsberg [:julienw]]

master: 1926e9d063b024748e13eb0aa347f18ee57a6cb4

Comment 12

[Julien Wajsberg [:julienw]]

This possibly made the event fluffing algorithm upset (I mean: it's more difficult to focus the composer now).

NI me to test this.

Comment 13

[:gerard-majax]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> This possibly made the event fluffing algorithm upset (I mean: it's more
> difficult to focus the composer now).
> 
> NI me to test this.

Looks like you were right: reverting this, I can much more easily focus on the composer text input field :(

Comment 14

[Julien Wajsberg [:julienw]]

Thanks for testing :)
I'd like to test with the patch in bug 946731 (hopefully it's not too much rotten)

Comment 15

[Julien Wajsberg [:julienw]]

reverted on master: 744f955addea27deab345fcffa67a0a3ca0a0a23

Sorry, I didn't have the time to work on bug 946731.
So I revert this, and we'll have to add specific listeners on the left and right parts instead of having a listener on the whole form like I suggested.

Sorry Rishav :(

Comment 16

[kumar rishav (:rishav_)]

Okay No problem :)
I will look into this soon .

Thanks

Comment 17

[kumar rishav (:rishav_)]

Attachment: Tapping on empty space above send and attach buttons should assimilate currently edited recipient

Hi Julien
Have a look on the PR. I have implemented left and right side separately. for left side, created a new div element. Hope there won't be any regression now.
Thanks

Comment 18

[Julien Wajsberg [:julienw]]

Comment on attachment 8528888 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient

Just some nits and I think that we'll be good.

Please add your new changes in a separate commit as it makes the next review easier :)

Hey Oleg, just NI you because I left a question for you on the pull request.

Comment 19

[kumar rishav (:rishav_)]

Comment on attachment 8528888 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient

Hi Julienw
Updated the patch.
Left comment on github about the ni? (oleg), as far as i checked.
Thanks

Comment 20

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #18)
> Comment on attachment 8528888 [details] [review]
> Tapping on empty space above send and attach buttons should assimilate
> currently edited recipient
> 
> Hey Oleg, just NI you because I left a question for you on the pull request.

Replied on Github.

Comment 21

[kumar rishav (:rishav_)]

Comment on attachment 8528888 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient

Hi julienw
Updated the PR and had the discussion with oleg on this. Have a look on this.
Thanks

Comment 22

[Julien Wajsberg [:julienw]]

Comment on attachment 8528888 [details] [review]
Tapping on empty space above send and attach buttons should assimilate currently edited recipient

r=me if you correct the latest small nits

When you've finished, you can squash your commits, push to the pull request, and ask "checkin-needed" :)

Thanks a lot !

Comment 23

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/39214fb22c203e8849aaa1c27b773eeb73212921