961950 - [Sora][Message]The "message setting" menu should in "Messages"main screen.

Status: RESOLVED FIXED

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

Description

[sync-1]

Mozilla build ID: 20140105004001
 DEFECT DESCRIPTION:
 ->The"Setting"should in "Messages"main screen.
 
  REPRODUCING PROCEDURES:
 ->Enter"Messages"to edit a message,there is have"Setting"menu;
 
  EXPECTED BEHAVIOUR:
 ->The"Setting"should in "Messages"main screen.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:100%
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[Julien Wajsberg [:julienw]]

Actually, see UX Design attachment 8337617 [details] page 40 and 42, we should have a "settings" option in the thread list view too.

Steve, maybe you can take this bug?

Comment 2

[Steve Chung [:steveck]]

I can take a look for sure.

Comment 3

[Steve Chung [:steveck]]

Attachment: Link to github

Hi Borja, I replace the icon in the thread list view and sync with thread view page. Please have a look when you have free cycle.

Hi Jose, I have a small concern about the detail. When there is no message in the app, should we enable the option icon and only shows settings in the menu? I disable option button when empty, but it also makes sense that we keep settings enable whether message exist or not.

Comment 4

[Borja Salguero [:borjasalguero]]

Comment on attachment 8366446 [details] [review]
Link to github

@Steve: Some comments in the PR! Ask me again to review this when you will be ready!

@Ayman: Some details are related with interaction, could you take a look? PR is https://github.com/mozilla-b2g/gaia/pull/15753 .Thanks!

Comment 5

[José Vittone]

(In reply to Steve Chung [:steveck](OOO from 1/30 - 2/4) from comment #3)
> Hi Jose, I have a small concern about the detail. When there is no message
> in the app, should we enable the option icon and only shows settings in the
> menu? I disable option button when empty, but it also makes sense that we
> keep settings enable whether message exist or not.
For me it make sense. I would say that we need a [more] icon next to compose, and then show both options "Edit" and "Settings" in an option menu. 

@Ayman: What do you think?

Comment 6

[ayman maat :maat]

(In reply to José Vittone from comment #5)
> (In reply to Steve Chung [:steveck](OOO from 1/30 - 2/4) from comment #3)
> > Hi Jose, I have a small concern about the detail. When there is no message
> > in the app, should we enable the option icon and only shows settings in the
> > menu? I disable option button when empty, but it also makes sense that we
> > keep settings enable whether message exist or not.
> For me it make sense. I would say that we need a [more] icon next to
> compose, and then show both options "Edit" and "Settings" in an option menu. 
> 
> @Ayman: What do you think?

fine by me, but could we call 'Edit' 'Delete' please because the user is deleting the messages, not editing them

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8366446 [details] [review]
Link to github

Hi Borja, I've updated the patch with new flow based on previous proposal for empty list case. Please have a look for the new patch and tests, thanks!

Comment 8

[Steve Chung [:steveck]]

Comment on attachment 8366446 [details] [review]
Link to github

Reassign the reviewer since there has no update for week.
Hi Julien, Borja already gave some comments before, and I hope this small patch wouldn't block you for a long time...

Comment 9

[Borja Salguero [:borjasalguero]]

Hi Steve, sorry for the delay, Im now back in Gaia so I can take this review right now (that's why I asked for rebasing the patch :) )

My only concern about this patch is that we are loosing the capacity of enabling/disabling the 'edit' button when a new message arrives. If the option menu is shown with an empty list, if a new message arrives we can not enable the button properly.

I dont know if we could hide this prompt when a new message arrives, or if we can create another OptionMenu on the fly removing the old and non-up-to-date one. Wdyt?

Comment 10

[Julien Wajsberg [:julienw]]

I think we should not show the item at all if the list is empty, and not try to add it when we receive the message. Let's keep it simple here, I don't see the value to update the menu on the fly, especially for an edge case like this.

Ayman, what do you think?

Comment 11

[ayman maat :maat]

Hey Julien

From a UX PoV this is how i think it should work:

**PATH 1**

1) open message app to view empty inbox
2) select the list  CTA in top right hand corner
(menu presented but delete CTA is disabled)
3) incoming message arrives

**EXPECTED**
Delete CTA remains disabled.

**RATIONAL**
If the user is viewing the list presented by the CTA in the top right hand corner of the inbox at the moment that a message is incoming the user is highly unlikely to want to delete the messages straight away as they will not know the content of the message that has just been received.

—————————

**PATH 2**

1) open message app to view empty inbox
2) select the list CTA in top right hand corner
(menu presented but delete CTA is disabled)
3) close list to view empty inbox
4) incoming message arrives
5) select the list CTA in top right hand corner

**EXPECTED**
Delete CTA is now enabled.

**RATIONAL**
The user has had the chance to review the content of the incoming message and is more likely to want to delete all messages.

ni? me if you require more input

Comment 12

[Julien Wajsberg [:julienw]]

Perfect, thanks!

Comment 13

[Julien Wajsberg [:julienw]]

Comment on attachment 8366446 [details] [review]
Link to github

Giving the review back to Borja now that he's back ;)

Steve, if Borja is still too slow (:p) please redirect to me!

Comment 14

[Borja Salguero [:borjasalguero]]

Thanks Julien! Re-testing and sorry (again) for the delay!

Comment 15

[Borja Salguero [:borjasalguero]]

Comment on attachment 8366446 [details] [review]
Link to github

R+!

Comment 16

[Borja Salguero [:borjasalguero]]

https://github.com/mozilla-b2g/gaia/commit/de41d2e59f4510448c7d12f68b40cd114989509b
https://github.com/steveck-chung/gaia/commit/4b70e8c0969e3b5c12230adfdee5562426f343c0

Sorry again for the delay! R+ and merged!

Comment 17

[田旻]

Will This bug be fixed on v1.3?

Comment 18

[Jason Smith [:jsmith]]

Minor bug, not a blocker for 1.3.