999944 - [SMS] The cursor in the To field is shown in second place but the letter entered is added before the cursor(in first place)

Status: RESOLVED FIXED

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

Description

[sasikala]

1. Title : The cursor in the To field is shown one space gap and the first letter entered is added before the cursor
2. Precondition : 1. SMS app should be working.
3. Tester's Action : 
      1)Open SMS application
      2)Select New Message icon
      3)Check the cursor position in the 'To:' field.

4. Expected : The letter entered should be added after the cursor

5. Actual :  The cursor is placed in the second place but the letter/number entered is added in the first place(before the cursor)

Comment 1

[sasikala]

Attachment: Cursor_image.png

Image attached.

Comment 2

[ankit93040]

Please see above!

Comment 3

[ankit93040]

Hi Julien & Steve

Message -> New Message -> To Field

Another Issue is this - Enter number say 12345 & then a ;.
12345 is added as a recipient. Now press backspace. Now cursor is shown where - 12345 followed by one space & then cursor is shown. right? At these the cursor is not touching 5 of 12345. But if we press backspace here then 5 is deleted & after that cursor position touches 4 of 12345.

I think this two are related.

Plese comment!

Comment 4

[ankit93040]

Attachment: second.png

Pic for comment # 3.

Comment 5

[ankit93040]

A little more observation on these - 

Message -> New Message -> TO Field

Automatic cursor will be pointed & keyboard will be shown. 

Observe the cursor position & Press one time space bar. Notice that the cursor moves back instead of front.

Now if you have pressed space only once then press a backspace key & Observe the cursor. It moves front instead of back.

Comment 6

[Steve Chung [:steveck]]

For the comment 0, I think it's matter of stying. You could try to adjust the empty & focus recipient style here: https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/style/sms.css#L946
Just adjust the padding: 0 0.8rem to padding-right: 0.8rem and it should solve your concern. About comment 3, I'm not quite sure it's another issue or not. Maybe we need more time for investigation.

Comment 7

[ankit93040]

Hi Steve

Thanks for your quick response.

Comment # 1 & Comment # 5 issue is resolved with your changes.

However comment # 3 issue still exists.

so do I need to raise a new a bugzilla issue for issue in comment # 3? or we will go head with these.

I'll create a pull request & get these issue fixed.

Comment 8

[ankit93040]

Attachment: 99944.patch

Hi

These bugzilla has total 3 issues as mentioned in comment #0, #3, #5.

The above patch solves two issues namely in comment #0 and #5.

Comment #3 is still an issue.

I'll raise a new bugzilla for that issue.

Kindly review the same.

Thanks!

Comment 9

[Julien Wajsberg [:julienw]]

Be careful with the padding styling, please test carefully all cases:
* invalid recipients
* valid recipients
* editing recipients
* long recipients (longer than the available space) for all these 3 cases

Comment 10

[ankit93040]

Hi Julien


All seems to be okay as mentioned by you in comment # 9.

Comment 11

[Victoria Gerchinhoren [:vicky]]

Hey if there's any UI to be reviewed in this patch can you please attach a screenshot of the instance to be visually reviewd? Thanks!

Comment 12

[ankit93040]

Hi Victoria

Original:- Message -> New Message -> Automatic cursor is shown in To Field. The cursor is shown in the second place not the first place because of which if we add any first character to To Field it seems to be going left of cursor.

After the patch:- Message -> New Message -> Automatic cursor is shown in To Field. The cursor is shown in the first place because of which if we add any first character to To Field it will go to left of cursor.

Please notice the difference in cursor position. May be you need to add one character & notice the way its added.

Please take a look at Cursor_image.png attached above for your reference.

Comment 13

[Victoria Gerchinhoren [:vicky]]

(In reply to ankit93040 from comment #12)
> Hi Victoria
> 
> Original:- Message -> New Message -> Automatic cursor is shown in To Field.
> The cursor is shown in the second place not the first place because of which
> if we add any first character to To Field it seems to be going left of
> cursor.
> 
> After the patch:- Message -> New Message -> Automatic cursor is shown in To
> Field. The cursor is shown in the first place because of which if we add any
> first character to To Field it will go to left of cursor.
> 
> Please notice the difference in cursor position. May be you need to add one
> character & notice the way its added.
> 
> Please take a look at Cursor_image.png attached above for your reference.

Yes, but how does it look with you patch? Thanks

Comment 14

[ankit93040]

Hi Victoria

It looks the same as without my patch. Only difference is with respect to cursor position. After my patch Cursor is shown in the first place instead of second place.

Observe the difference between the cursor position with/without my patch.

Without my patch the cursor position is one step ahead.
With my patch the cursor is at the correct position.
You might need to type a (only one) character to check the difference in the position of cursor.
Please observer closely as how the cursor position changes upon entering a character.

Without my patch when you enter a character the cursor position doesn't change & the character goes towards left of cursor.

With my patch when you enter a character the cursor position change & the character goes towards right of cursor.

If I am still unclear let me know. 

thanks!

Comment 15

[Victoria Gerchinhoren [:vicky]]

(In reply to ankit93040 from comment #14)
> Hi Victoria
> 
> It looks the same as without my patch. Only difference is with respect to
> cursor position. After my patch Cursor is shown in the first place instead
> of second place.
> 
> Observe the difference between the cursor position with/without my patch.
> 
> Without my patch the cursor position is one step ahead.
> With my patch the cursor is at the correct position.
> You might need to type a (only one) character to check the difference in the
> position of cursor.
> Please observer closely as how the cursor position changes upon entering a
> character.
> 
> Without my patch when you enter a character the cursor position doesn't
> change & the character goes towards left of cursor.
> 
> With my patch when you enter a character the cursor position change & the
> character goes towards right of cursor.
> 
> If I am still unclear let me know. 
> 
> thanks!

Thanks Ankit (?) 
I can put a plus, if it's working properly now, but I review UI visual implementation, so for this kind of things you don't need to set a flag to me. But when I review UI for badwidth reasons I need a screenshot.

Thanks,

Comment 16

[Julien Wajsberg [:julienw]]

Comment on attachment 8410935 [details] [diff] [review]
99944.patch

Yep, no need to waste time from the VD team, the peers can handle this.

Please request review when you're ready. I think Steve commented so you can ask him.

Comment 17

[Kevin Grandon :kgrandon]

Assigning this to Ankit as requested over IRC. Good luck :)

Comment 18

[ankit93040]

Attachment: Pointer to Pull Request.html

Hi steve

As mentioned in comment # 16 it doesn't require UI/UX review.
As also mentioned in comment # 9 it passes all well.

Kindly merge it to master.

Thanks!

Comment 19

[Julien Wajsberg [:julienw]]

Steve, please check correctly comment 9 + the STR in bug 1000084 comment 5. Thanks!

Comment 20

[Julien Wajsberg [:julienw]]

I remember the padding were carefully choosen for this, so maybe if we remove some padding we would need to add some margin? Just thinking...

Comment 21

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] from comment #19)
> Steve, please check correctly comment 9 + the STR in bug 1000084 comment 5.
> Thanks!

I've tried comment 9 before and not yet for bug 1000084 comment 5, maybe the difference would become the empty focusing recipient start from original line or next line. Ankit, could you help with verifying the reproducing step in bug 1000084 comment 5 and see if behavior changes. Adding margin or adjusting the right padding should be reasonable change(if only changing left padding does cause some regression), and I would like to have some comment in css especially for some specific tuning in the future...

Comment 22

[ankit93040]

Dear Steve

In reply to comment # 21 - "Ankit, could you help with verifying the reproducing step in bug 1000084 comment 5 and see if behavior changes." , steve.

There is absolutely no change in behavior either with/without my patch.

Comment 23

[Julien Wajsberg [:julienw]]

will check as well

Comment 24

[Julien Wajsberg [:julienw]]

Oki, this is fine from my point of view, thanks !

Comment 25

[Julien Wajsberg [:julienw]]

Comment on attachment 8411674 [details]
Pointer to Pull Request.html

r=me

Comment 26

[Julien Wajsberg [:julienw]]

Ankit, can you please add "r=julien" in the first line of your commit log, and then ping me when you did it?

Comment 27

[ankit93040]

Added commit message with r=julein. Please check the above pull request.

thanks!

Comment 28

[Julien Wajsberg [:julienw]]

I'm sorry ankit, seems like you need to rebase.

Comment 29

[ankit93040]

Attachment: Pointer to Pull Request.html

Hi Julienw

Find the updated & rebased pull request.

Please merge to master.

Thanks!

Comment 30

[Julien Wajsberg [:julienw]]

Comment on attachment 8414412 [details]
Pointer to Pull Request.html

landed in master: 48a646c15836f61181ee4bb297c1b6b78ea86335
then reverted because I saw the Bug number was missing: b76416824b114730cc6c209842c7cf168faf7ee3

then Relanded: 01025db45330a8a7dfa9b1f4da326d0110b62a69