[MMS/SMS] The dialog when tapping on a contact into ‘To’ field is not correct

VERIFIED FIXED in Firefox OS v1.1hd

Status

Firefox OS
Gaia::SMS
P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: isabelrios, Assigned: arcturus)

Tracking

unspecified
1.1 QE4 (15jul)
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 verified, b2g-v1.1hd fixed)

Details

(Whiteboard: MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0], leorun4,[LeoVB+] )

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 759667 [details]
Removing a recipient dialog

Bug seen on Unagi v1-train
Gecko-65bbcee
Gaia-13c6246

PROCEDURE
1.Start creating a SMS or MMS.
2.Tap on ‘+’ to add one recipient from contacts
3.Once the contact is into ‘To’ field, tap on it
4.Check the dialog shown

EXPECTED
The dialog shown should be the specified on page 9 of wireframe pack HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0. 

ACTUAL
Current dialog message does not show info about the contact’s number and it is not matching the specs. Please see screenshot attached

For a better understanding of the impact of this screen here is the UX feedback:
1)	Disambiguation #1. If the user has more than one ‘Aaaa’ in their address book it gives them the opportunity to understand which ‘Noe’ the message is being sent to
2)	Disambiguation  #2. (this is the most important disambiguation) If the ‘Aaaa’ contact has more than one phone associated to it, this screen should allow the user to know which ‘Aaaa‘ phone has been added to the ‘to’ field. If the phone information is not added to this screen there is absolutely no opportunity for the end user to disambiguate which ‘Aaaa’ phone has been added to the ‘to’ field

Comment 1

5 years ago
Ayman, can you please confirm the spec and expected behavior? Thanks!
Flags: needinfo?(aymanmaat)
Assignee: nobody → francisco.jordano

Comment 2

5 years ago
(In reply to Stephany Wilkes from comment #1)
> Ayman, can you please confirm the spec and expected behavior? Thanks!

I can confirm Isabel is correct in her description that this functionality should be implemented as specified in the wireframes for reasons of disambiguation and reduction of end user cognitive loading.
Flags: needinfo?(aymanmaat)
Depends on: 881248
Sorry about that, my initial understanding was incorrect.
No longer depends on: 881248
Hi,

Ayman I was doing the proper changes, propagating the information we need (we were just missing the phone type, and were not using the phone number).

But realised that we are using a confirm to send the message. In that case we can setup just a title, so the buttons wont be 'Remove' or 'Cancel', but the standards 'Cancel' and 'OK'.

As well we can specify one line (ok, we can break the line) but is a single text.

You can take a look to the poc:

http://gyazo.com/f1b75e9befd6d1e3eeee16060ea79c55

Also in the branch:

https://github.com/arcturus/gaia/tree/bug-880624

I would suggest to do a wording to fit in a confirm, but as you comment add the extra information to disambiguate some cases.

Thanks!
Flags: needinfo?(aymanmaat)
@Francisco, ugh that is a serious can of worms.
@Rick, yup, we will need a clear confirmation of what we can do.

Thx!

Comment 7

5 years ago
This issue tested on Leo 1.1 (Test Run Firefox OS 1.1.0 Full – Leo)
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8e3f39363c54
Gaia: ea18de80fd04110756becaed214656642388401d
Platform Version: 18.0

STEPS:
1.Start creating a SMS or MMS.
2.Tap on ‘+’ to add one recipient from contacts
3.Once the contact is into ‘To’ field, tap on it
4.Check the dialog shown

EXPECTED
The dialog shown should be the specified on page 9 of wireframe pack HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0. : 
Contact info:
- first name
- last name
- type
- phone number
Options:
- Remove
- Cancel

ACTUAL
Current dialog message does not show correct options and labels as defined in the corresponding specs. Ask the user about removing the contact. And the two only options are "Cancel" and "OK"

Link to failed test case:
https://moztrap.mozilla.org/manage/case/7916/
https://moztrap.mozilla.org/manage/case/7917/
https://moztrap.mozilla.org/manage/case/7918/
https://moztrap.mozilla.org/manage/case/7969/
Whiteboard: MMS_TEF → MMS_TEF, leorun3
The spec looks clear. Blocking+ for now.

If carrier/hardware think we should not block, can renominate.
blocking-b2g: leo? → leo+
Depends on: 883911
Not so sure about the hard dependency on bug 883911, it's true that we need to be clear on the exact way of display all those fields.

I'm more worry about the fact that the spec is a disambiguation screen (you can have acontact with several numbers and that helps you to discern which one are you texting) that doesn't fit the 'confirm' model.

Anyway I'll try to figure out with Ayman if we can fit that 'disambiguation' on a confirm dialog or not.

Cheers!
F.
After talking with Ayman and seeing what we can do with the CustomDialog class coming from the shared components, that is already being used, I totally agree with Rick (not like my previous comment), that we will have the dependency on bug 883911 to be sure we display the same information across all parts of the application.
Created attachment 764847 [details]
Pointer to PR 10491



Disambiguation screen as suggested by Ayman

As we are using the CustomDialog coming from shared, we have two main components:
title: We used the display name
message: We are using the text showed to disambiguate the contact in the contacts suggestions

Also the semantic of the CustomDialog forces us to put a 'negative' option on the right hand side (instead of the left) as was in the wf.

All this has been reviewed and approved by Ayman.

Victoria I'm adding you as needinfo cause the 'negative' color for the 'Remove' button is dark orange, and would like to be sure that's the way we do it here.

Thanks.
Attachment #764847 - Flags: review?(waldron.rick)
Attachment #764847 - Flags: feedback?(vpg)
Comment on attachment 764847 [details]
Pointer to PR 10491

Not needed anymore since we will stop using custom dialog to implement the original specs
Attachment #764847 - Flags: feedback?(vpg)
No longer depends on: 883911
(In reply to Rick Waldron from comment #13)
> https://github.com/mozilla-b2g/gaia/pull/10491/files#0
> 
> Once this is fixed, r=me

Hi Rick,

thanks for taking another look, but your last comment is not clear to me, it seems that the link is not pointing me to what you want me to fix.

Are you talking about 'unfocusing' the keyboard, you want the focus back?

Thanks,
F.
Attachment #764847 - Flags: review?(fbsc)

Updated

5 years ago
Blocks: 880257
Comment on attachment 764847 [details]
Pointer to PR 10491

Evan, this is the new generic dialog, could you take a look? Thanks!
Attachment #764847 - Flags: feedback?(evanxd)
Hi Borja,

Cool.
Hope we could land the patch today. :)

Thanks.
Comment on attachment 764847 [details]
Pointer to PR 10491

Thanks for the patch Francisco!
Attachment #764847 - Flags: review?(waldron.rick)
Attachment #764847 - Flags: review?(fbsc)
Attachment #764847 - Flags: review+
Attachment #764847 - Flags: feedback?(evanxd)
I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 72565090cb39dc65d136027769018a76cd61041b
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(francisco.jordano)
Borja, 

There are a lot of issues with this patch:

1. It duplicates too much functionality 
  - OptionMenu and Dialog can definitely be merged into a single entity
  - Utils.getContactDisplayInfo should use Utils.getCarrierTag

2. There are several changes that have nothing to do with the bug

3. Utils.getContactDisplayInfo reintroduces a nasty bug that was fixed elsewhere. The code that reintroduces this issue is actually unnecessary since it only duplicates the functionality of Utils.getCarrierTag
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

5 years ago
Flags: needinfo?(aymanmaat)
Whiteboard: MMS_TEF, leorun3 → MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0]

Comment 21

5 years ago
This issue tested on Leo 1.1 (Test Run Firefox OS 1.1.0 Full – Leo Run 4)
Gecko a378807ff04076c20f08b0102286b9eb2d08d60a
Gaia 1436e2778b90bd74635b0b94d1cf8ccb0d71b60c
Platform Version: 18.1

STEPS:
1.Start creating a SMS or MMS.
2.Tap on ‘+’ to add one recipient from contacts
3.Once the contact is into ‘To’ field, tap on it
4.Check the dialog shown

EXPECTED
The dialog shown should be the specified on page 9 of wireframe pack HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0. : 
Contact info:
- first name
- last name
- type
- phone number
Options:
- Remove
- Cancel

ACTUAL
Current dialog message does not show correct options and labels as defined in the corresponding specs. Ask the user about removing the contact. And the two only options are "Cancel" and "OK"

Link to failed test case:
https://moztrap.mozilla.org/manage/case/7918/
https://moztrap.mozilla.org/manage/case/7916/
https://moztrap.mozilla.org/manage/case/7917/
https://moztrap.mozilla.org/manage/case/7969/
Whiteboard: MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0] → MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0], leorun4
(In reply to bov from comment #21)
> This issue tested on Leo 1.1 (Test Run Firefox OS 1.1.0 Full – Leo Run 4)
> Gecko a378807ff04076c20f08b0102286b9eb2d08d60a
> Gaia 1436e2778b90bd74635b0b94d1cf8ccb0d71b60c
> Platform Version: 18.1
> 
> STEPS:
> 1.Start creating a SMS or MMS.
> 2.Tap on ‘+’ to add one recipient from contacts
> 3.Once the contact is into ‘To’ field, tap on it
> 4.Check the dialog shown
> 
> EXPECTED
> The dialog shown should be the specified on page 9 of wireframe pack
> HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0. : 
> Contact info:
> - first name
> - last name
> - type
> - phone number
> Options:
> - Remove
> - Cancel
> 
> ACTUAL
> Current dialog message does not show correct options and labels as defined
> in the corresponding specs. Ask the user about removing the contact. And the
> two only options are "Cancel" and "OK"
> 
> Link to failed test case:
> https://moztrap.mozilla.org/manage/case/7918/
> https://moztrap.mozilla.org/manage/case/7916/
> https://moztrap.mozilla.org/manage/case/7917/
> https://moztrap.mozilla.org/manage/case/7969/

Sorry, this is still not uplift to v1-train yet
Hi Rick,

this landed and some bugs were dependant of this. Some of the functions that you comment are not working in the way the should (like having a single way displaying all the info, standarised).

Again, totally agree with you, and taking into account that is already landend in master and has already a hard dependency for bug 880257. Let's not reopen the bug and fill a new one to address all the issues you want to fix (will be happy to fix them following your criteria)

Thanks!!
F.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Flags: needinfo?(francisco.jordano)
Resolution: --- → FIXED
I agree that we should land this, and if you consider that it's something pending here please file a bug. Thanks!
(Reporter)

Updated

5 years ago
Blocks: 890847
@gaye already uplift to v1-train.

Cannot find the flag to say it's fixed in v1-train, do we need to add anything else to move this to v1.1-hd, or it's ok now?
status-b2g18: --- → fixed

Comment 26

5 years ago
Tested on Unagi device.
Build: user.manifest.V1-train.Rel0.4.Sprint12.B-290.Gecko-e78450a.Gaia-7c40bda
Commercial RIL: 152

 STEPS:
1.Start creating a SMS or MMS.
2.Tap on ‘+’ to add one recipient from contacts
3.Once the contact is into ‘To’ field, tap on it
4.Check the dialog shown
 
EXPECTED
The dialog shown should be the specified on page 9 of wireframe pack
HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0. : 
Contact info:
- first name
- last name
- type
- phone number
 Options:
- Remove
- Cancel

ACTUAL
It works as expected
Status: RESOLVED → VERIFIED

Updated

5 years ago
blocking-b2g: leo+ → leo?
Priority: -- → P1
(In reply to Rick Waldron from comment #27)
> Further issues created by this patch:
> https://github.com/mozilla-b2g/gaia/commit/
> 8b41ee9a80c1d6b3613339e7388035d3fbcc575d#L2R227 created the issue described
> here: https://github.com/mozilla-b2g/gaia/pull/10771#issuecomment-20734398

Rick, please file a new bug and nominate it if you believe it is critical.
blocking-b2g: leo? → leo+
Flags: needinfo?(waldron.rick)

Updated

5 years ago
Target Milestone: --- → 1.1 QE4 (15jul)
Hi RyanVM, gaye, can you help and address the comment 25?

(In reply to Francisco Jordano [:arcturus] from comment #25)
> @gaye already uplift to v1-train.
> 
> Cannot find the flag to say it's fixed in v1-train, do we need to add
> anything else to move this to v1.1-hd, or it's ok now?

Rick, 
can you file the found bugs and describe if they are critical? Is it worth Leo taking the patch here without the new problems being fixed? (or should we back this one out if the new bugs cannot be fixed in time?)
Flags: needinfo?(ryanvm)
Flags: needinfo?(gaye)
Sorry, answer myself here. b2g-18 denotes v1-train.

Leaving ni?rick for comment 28 and comment 29
Flags: needinfo?(ryanvm)
Flags: needinfo?(gaye)
Hi all,
Rick's comment is about a scenario when adding a contact which has been identified and fixed in bug https://bugzilla.mozilla.org/show_bug.cgi?id=888150. As this bug is leo+ as well, the whole functionality will be ready soon. No need to file a new bug due to the reasons explained here. Thanks!
Actually bug https://bugzilla.mozilla.org/show_bug.cgi?id=888150 has been landed, so the whole functionality is working as expected.
Flags: needinfo?(waldron.rick)

Updated

5 years ago
status-b2g18: fixed → verified

Updated

5 years ago
Whiteboard: MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0], leorun4 → MMS_TEF, leorun3, [u=commsapps-user c=messaging p=0], leorun4,[LeoVB+]
v1.1.0hd: 7c40bdaeaffae708342fc773926dcfac5389348e
status-b2g-v1.1hd: --- → fixed
You need to log in before you can comment on or make changes to this bug.