Closed
Bug 923024
Opened 11 years ago
Closed 11 years ago
[MMS] Add options menu button on top bar
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:1.3+)
People
(Reporter: fcampo, Assigned: fcampo)
References
Details
Attachments
(4 files)
New specs need a message settings button on the top bar, that will give us the chance to add a subject to the mms
Comment 1•11 years ago
|
||
(In reply to Fernando Campo (:fcampo) from comment #0)
> New specs need a message settings button on the top bar, that will give us
> the chance to add a subject to the mms
Is there a visual spec for this? Thanks!
Flags: needinfo?(fernando.campo)
Assignee | ||
Comment 2•11 years ago
|
||
Same as in bug 923023 (as all are part of the bug 885680, unfinished specs and wf, unfinished visuals. Mostly inferred from current style and first version wf.
Flags: needinfo?(fernando.campo)
Comment 3•11 years ago
|
||
Ok, cool. As with 886680, let's hold off on actual changes to the app UI until the designs are finalized.
Updated•11 years ago
|
Target Milestone: --- → 1.2 QE1(Oct11)
Assignee | ||
Comment 4•11 years ago
|
||
Final wireframes and visuals posted on bug 885680
This is only first proposal, as there's things on the wf that have no acceptance yet (e.g. settings)
Attachment #814157 -
Flags: review?(waldron.rick)
Comment 5•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
ThreadUI.showOptions needs tests, please r? me when ready. Thanks! :)
Attachment #814157 -
Flags: review?(waldron.rick) → review-
Comment 6•11 years ago
|
||
(In reply to Fernando Campo (:fcampo) from comment #4)
> Created attachment 814157 [details] [review]
> Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
>
> Final wireframes and visuals posted on bug 885680
> This is only first proposal, as there's things on the wf that have no
> acceptance yet (e.g. settings)
Right, it's not clear what things should be in this settings menu.
Comment 7•11 years ago
|
||
(In reply to Rick Waldron from comment #6)
> (In reply to Fernando Campo (:fcampo) from comment #4)
> > Created attachment 814157 [details] [review]
> > Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
> >
> > Final wireframes and visuals posted on bug 885680
> > This is only first proposal, as there's things on the wf that have no
> > acceptance yet (e.g. settings)
>
> Right, it's not clear what things should be in this settings menu.
Adding UX to ni in order to clarify which options will be gathered under that menu, as far as I know just the icon itself is pending to be concreted.
Flags: needinfo?(aymanmaat)
Comment 8•11 years ago
|
||
(In reply to Rick Waldron from comment #6)
> (In reply to Fernando Campo (:fcampo) from comment #4)
> > Created attachment 814157 [details] [review]
> > Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
> >
> > Final wireframes and visuals posted on bug 885680
> > This is only first proposal, as there's things on the wf that have no
> > acceptance yet (e.g. settings)
>
> Right, it's not clear what things should be in this settings menu.
Hi Rick - The term 'Settings' that is being used for this CTA is something of a misrepresentation now (its a throwback to a much earlier concept).
referring to FFOS_MessageSubject_V1.3_20121007_V2.0 which is attached to bug 885680
page 5 details the options under the CTA as 'Add/Remove Subject' and 'Settings'
page 7 details the options as Add/Remove Subject', 'delete messages' and 'Settings'
The options under these menu are being added to in other bugs.
...so really the CTA is a 'menu' and certainly not 'settings'.
From a UX PoV i am not happy with the mixed bag of functions and entry points to functional lists (settings) being lumped together in a single list as i am prescribing... but APU design wise my hands are tied by our current Patterns and our timelines as to propose a more sexy solution would blow the timeline for both Dev and Design as it would impact more than just the messaging app and would require new patterns.
Flags: needinfo?(aymanmaat)
Comment 9•11 years ago
|
||
(In reply to ayman maat :maat from comment #8)
> (In reply to Rick Waldron from comment #6)
> > (In reply to Fernando Campo (:fcampo) from comment #4)
> > > Created attachment 814157 [details] [review]
> > > Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
> > >
> > > Final wireframes and visuals posted on bug 885680
> > > This is only first proposal, as there's things on the wf that have no
> > > acceptance yet (e.g. settings)
> >
> > Right, it's not clear what things should be in this settings menu.
>
> Hi Rick - The term 'Settings' that is being used for this CTA is something
> of a misrepresentation now (its a throwback to a much earlier concept).
>
> referring to FFOS_MessageSubject_V1.3_20121007_V2.0 which is attached to bug
> 885680
> page 5 details the options under the CTA as 'Add/Remove Subject' and
> 'Settings'
> page 7 details the options as Add/Remove Subject', 'delete messages' and
> 'Settings'
> The options under these menu are being added to in other bugs.
>
> ...so really the CTA is a 'menu' and certainly not 'settings'.
>
> From a UX PoV i am not happy with the mixed bag of functions and entry
> points to functional lists (settings) being lumped together in a single list
> as i am prescribing... but APU design wise my hands are tied by our current
> Patterns and our timelines as to propose a more sexy solution would blow the
> timeline for both Dev and Design as it would impact more than just the
> messaging app and would require new patterns.
Ayman, thanks for these details—certainly helps illustrate a better picture for these changes :)
Updated•11 years ago
|
Target Milestone: 1.2 QE1(Oct11) → 1.3 Sprint 2 - 10/11
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
Tests added :)
Attachment #814157 -
Flags: review- → review?(waldron.rick)
Comment 11•11 years ago
|
||
Ayman, while reviewing this I noticed that the menu displays "Delete Messages" and when you tap that option, it brings you to a page that whose header reads "Edit Messages". Should they match? If yes, which is more appropriate? I'm inclined to change the header to read "Delete Messages", but will await your decision. Thanks!
Flags: needinfo?(aymanmaat)
Comment 12•11 years ago
|
||
(In reply to Rick Waldron from comment #11)
> Ayman, while reviewing this I noticed that the menu displays "Delete
> Messages" and when you tap that option, it brings you to a page that whose
> header reads "Edit Messages". Should they match? If yes, which is more
> appropriate? I'm inclined to change the header to read "Delete Messages",
> but will await your decision. Thanks!
Good Spot Rick
Totally. We need to align language of the CTA and the title in the header. I would also prefer to change the header to read "Delete Messages" as that is what is happening. I have never understood why the word "Edit" is used when there is a more clearer and definitive word available.
I am going to ni? Tyler (copywriter) to confirm he is happy with this language change. If he is happy with it, lets do it.
Flags: needinfo?(aymanmaat) → needinfo?(tyler.altes)
Comment 13•11 years ago
|
||
Yep, sounds good to me. If the only thing happening on the screen is deleting or not deleting messages, the title should be "Delete Messages".
Flags: needinfo?(tyler.altes)
Assignee | ||
Comment 14•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
I rebased the PR and updated it with the string changes, so asking for review also to someone from l10n-team.
As this is not a blocker, and we are on time for 1.3, not sure if I should use the tag late-l10n or not.
Attachment #814157 -
Flags: ui-review?
Attachment #814157 -
Flags: review?(l10n)
Comment 15•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
string IDs should be treated like variable names, in this case, not edit2, but something like deleteMessages. Which you already add, though.
What's the difference between the two strings? If they're using in different contexts, it'd be good to tell the difference.
Also, I don't think the comment about bug 885680 should be in the l10n files.
Attachment #814157 -
Flags: review?(l10n) → review-
Comment 16•11 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #15)
> Comment on attachment 814157 [details] [review]
> Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
>
> string IDs should be treated like variable names, in this case, not edit2,
> but something like deleteMessages. Which you already add, though.
>
> What's the difference between the two strings? If they're using in different
> contexts, it'd be good to tell the difference.
Agreed, instead of "editMode2", just share the "deleteMessages" string
Assignee | ||
Comment 17•11 years ago
|
||
Sure. I just wanted to keep the logic division between 'Headers' and 'Settings options' (which I will rename to 'Options menu'), but if you'd rather to use the same, no prob.
Comment 18•11 years ago
|
||
If the same string is used in different visual contexts, we should use different IDs. Localizers might need to shorten one translation more brutally than the other.
Assignee | ||
Comment 19•11 years ago
|
||
So in this case, I think they are different visual contexts.
Former 'editMode' is used for the header of the screen, which we want to change now from 'Edit mode' to 'Delete messages' while 'deleteMessages' is used for a specific option from a menu.
I agree that in english is exactly the same screen, but maybe in other languages we need to use different ones (one is a description of the screen, the other is an action)
I'm gonna wait for a little agreement on this before submitting the changes
Comment 20•11 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #18)
> If the same string is used in different visual contexts, we should use
> different IDs. Localizers might need to shorten one translation more
> brutally than the other.
But that will contradict the issue that Ayman and I hoped to address, won't it?
Comment 21•11 years ago
|
||
There can be multiple IDs for the same English string.
I have a hard time to find out what exactly the two use-cases are in the many bugs and images and pdfs.
Comment 22•11 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #21)
> There can be multiple IDs for the same English string.
>
> I have a hard time to find out what exactly the two use-cases are in the
> many bugs and images and pdfs.
When clicking the new Options menu, the user will see an option to "Delete Messages". Tapping on "Delete Messages" will put the app in "Delete Messages" view where the user can select all or some messages or threads to delete.
I've attached an image the shows the two places that this string will appear.
Assignee | ||
Comment 23•11 years ago
|
||
(In reply to Rick Waldron from comment #20)
> (In reply to Axel Hecht [:Pike] from comment #18)
> > If the same string is used in different visual contexts, we should use
> > different IDs. Localizers might need to shorten one translation more
> > brutally than the other.
>
> But that will contradict the issue that Ayman and I hoped to address, won't
> it?
IMO is exactly the opposite of a contradiction. As I understand, the issue here is to keep the strings align in meaning, not necessarily in writing. That is, we shouldn't have a menu option telling the user 'Delete messages' and after clicking it send him/her to a screen stating 'Edit mode', but it would be perfectly valid to use a button stating 'Click here to delete your messages' and after click open a window with a header with 'Choose messages to delete', as we keep the same idea on user's mind (I know, stupid strings, but shows my point)
If we maintain two different strings, one per context, it would be easier for translators to keep the alignment on the meaning, while differentiating the contexts. I understand that in english, both strings could be the same and keep the meaning, but maybe in other languages it's better to write them in a different way.
Another different thing is to keep using the label 'editMode' (now 'editMode2') or find a better one for the new meaning (maybe deletionMode?), as it would make the code easier to read.
Comment 24•11 years ago
|
||
(In reply to Fernando Campo (:fcampo) from comment #23)
> (In reply to Rick Waldron from comment #20)
> > (In reply to Axel Hecht [:Pike] from comment #18)
> > > If the same string is used in different visual contexts, we should use
> > > different IDs. Localizers might need to shorten one translation more
> > > brutally than the other.
> >
> > But that will contradict the issue that Ayman and I hoped to address, won't
> > it?
> IMO is exactly the opposite of a contradiction. As I understand, the issue
> here is to keep the strings align in meaning, not necessarily in writing.
> That is, we shouldn't have a menu option telling the user 'Delete messages'
> and after clicking it send him/her to a screen stating 'Edit mode', but it
> would be perfectly valid to use a button stating 'Click here to delete your
> messages' and after click open a window with a header with 'Choose messages
> to delete', as we keep the same idea on user's mind (I know, stupid strings,
> but shows my point)
>
> If we maintain two different strings, one per context, it would be easier
> for translators to keep the alignment on the meaning, while differentiating
> the contexts. I understand that in english, both strings could be the same
> and keep the meaning, but maybe in other languages it's better to write them
> in a different way.
Ok, the use case is much clearer now, thank you for elaborating here.
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 2 - 10/11 → 1.3 Sprint 3 - 10/25
Assignee | ||
Comment 25•11 years ago
|
||
Just renaming to avoid the use of term settings (as it is one of the options), and updating dependencies
Depends on: 924409
Summary: [MMS] Add settings button on top bar → [MMS] Add options menu button on top bar
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
Assignee | ||
Comment 26•11 years ago
|
||
I was wondering if after the discussion and explanations on the previous comments, we are closer to an agreement on the situation.
To recap a little, if we agree on different strings for different situations, the editMode string should be renamed into a new one, either editMode2 to warn the translators, or other one (deleteMessagesHeader?).
On the other hand, if we decide to go for one string for all the situations (header and menu option), we can use deleteMessages only.
*The PR hasn't been updated since last review, waiting for a decision on this before modifying it
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(waldron.rick)
Flags: needinfo?(l10n)
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 3 - 10/25 → 1.3 Sprint 4 - 11/8
Updated•11 years ago
|
blocking-b2g: --- → 1.3+
Comment 27•11 years ago
|
||
Comment 28•11 years ago
|
||
Assignee | ||
Comment 29•11 years ago
|
||
(In reply to Peter La from comment #28)
> Created attachment 823348 [details]
> Screenshot showing new iconography in context
Hi Peter, I see in the screenshot that there's no longer use of a separator. Is this intended?
Flags: needinfo?(pla)
Assignee | ||
Comment 30•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
Pull Request updated with the new icons and asking for review again on l10n after the latest comments.
Attachment #814157 -
Flags: ui-review?
Attachment #814157 -
Flags: review?(l10n)
Attachment #814157 -
Flags: review-
Comment 31•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
As flod mentioned, don't use edit2 to say delete.
The two uses have different contexts, one is a button, one is a title. I suggest deleteMessages.label and .title or so.
Also, AFAICT, https://developer.mozilla.org/en-US/Apps/Design/Content indicates that they're using title and sentence case, resp?
Attachment #814157 -
Flags: review?(l10n) → review-
Updated•11 years ago
|
Flags: needinfo?(l10n)
Comment 32•11 years ago
|
||
Hi Fernando,
Sorry I should have clarified. Don't worry about how the header background looks (ie. how it looks different from v1.2 and earlier). You just have to make sure the icons are in and appear in the right positions. The header stuff is something that is being worked on for 1.3 as a separate item.
Flags: needinfo?(pla)
Assignee | ||
Comment 33•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
ok, here we go again.
I've decided to go for deleteMessages-title and deleteMessages-label, because we create the option buttons using a automatic javascript component, and modifying that so we add a title or label attribute to the buttons would be more complicated than just add new strings. Hope is not a problem for l10n team
Attachment #814157 -
Flags: review- → review?(l10n)
Comment 34•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
Thanks, looks good to me, didn't dive deeper than worth an f+, though.
Attachment #814157 -
Flags: review?(l10n) → feedback+
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.3 Sprint 5 - 11/22
Comment 35•11 years ago
|
||
Rick, Could I help you with this review? After the f+ from l10n team I think that it's ready to be reviewed again. If on monday is still under 'review?' flag I'll try to help you cleaning the review queue! Thanks! :)
Comment 36•11 years ago
|
||
Comment on attachment 814157 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12716
r=me after rebase
Attachment #814157 -
Flags: review?(waldron.rick) → review+
Comment 37•11 years ago
|
||
@Fernando, could you rebase and land this patch? Thanks!
@Rick, Thanks for your review :)
Flags: needinfo?(waldron.rick) → needinfo?(fer.campo.garcia+bugzilla)
Assignee | ||
Comment 38•11 years ago
|
||
Rebased and merged on master 083aeeff0e9716e540f87d562fb7f284db3b5532, as the travis errors are unrelated.
Thanks for the help guys
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(fer.campo.garcia+bugzilla)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•