Update "add to home screen" dialog to meet new interaction and UX specs

RESOLVED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: benfrancis, Assigned: crdlc)

Tracking

({late-l10n, polish})

unspecified
2.1 S4 (12sep)
All
Gonk (Firefox OS)
late-l10n, polish

Firefox Tracking Flags

(b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(3 attachments, 4 obsolete attachments)

Summary: Update "add to homescreen" dialog to meet new interaction and UX specs → Update "add to home screen" dialog to meet new interaction and UX specs
Blocks: 941180
(Reporter)

Updated

4 years ago
No longer blocks: 945259
Peter, Eric - I'm assuming that we use the same dialog for add and edit cases, right? Currently we have the url field there as well and it's editable, are we making a conscious decision to remove it?
Flags: needinfo?(pdolanjski)
Flags: needinfo?(epang)
(In reply to Kevin Grandon :kgrandon from comment #1)
> Peter, Eric - I'm assuming that we use the same dialog for add and edit
> cases, right? Currently we have the url field there as well and it's
> editable, are we making a conscious decision to remove it?

Good question.  +Francis since he didn't have the url field in the interaction spec either.
Flags: needinfo?(pdolanjski) → needinfo?(fdjabri)
Blocks: 945259
No longer blocks: 941180
Yes, this was a conscious decision. I don't really feel that the URL field was necessary so the design felt simpler without it.
Flags: needinfo?(fdjabri)
Flags: needinfo?(epang)
Cristian - would you have some cycles to help us with this dialog UX? Thanks!
Flags: needinfo?(crdlc)
(Assignee)

Updated

4 years ago
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Flags: needinfo?(crdlc)
(Assignee)

Comment 5

4 years ago
Created attachment 8482236 [details]
Add link
(Assignee)

Comment 6

4 years ago
Created attachment 8482237 [details]
Add link with keyboard
(Assignee)

Comment 7

4 years ago
Created attachment 8482238 [details]
Edit link
(Assignee)

Comment 8

4 years ago
Created attachment 8482239 [details]
Edit link with keyboard
(Assignee)

Comment 9

4 years ago
Francis,

   I would like to show you the screenshots (add and edit UI) in order to know the correct titles, to confirm that url field disappears, etc.

Thanks a lot
Flags: needinfo?(fdjabri)
(Assignee)

Comment 10

4 years ago
Created attachment 8482242 [details]
Go to PR
(Reporter)

Comment 11

4 years ago
(In reply to Francis Djabri [:djabber] from comment #3)
> Yes, this was a conscious decision. I don't really feel that the URL field
> was necessary so the design felt simpler without it.

Not being able to edit the URL of a bookmark seems like a bit of a pain and a feature we'd be losing from 2.0 :(
(Assignee)

Comment 12

4 years ago
Comment on attachment 8482236 [details]
Add link

I know that we need the feedback from Francis for the title but I think that the UI is ready to review
Attachment #8482236 - Flags: ui-review?(epang)
(Assignee)

Updated

4 years ago
Attachment #8482242 - Attachment description: WIP → Go to PR
Attachment #8482242 - Flags: review?(kgrandon)
Comment on attachment 8482242 [details]
Go to PR

I think it's close, and the code looks good, but it appears there's a perma-failing marionette test. Can you fix browser_chrome_bookmark_web_result_test.js and re-flag me for review? Thanks!
Attachment #8482242 - Flags: review?(kgrandon)
The visuals seem to follow Eric's spec, but flagging Eric to confirm. 
As for the title, this should be "Add to Homescreen" according to the spec.
Flags: needinfo?(fdjabri)
Flags: needinfo?(epang)
(In reply to Francis Djabri [:djabber] from comment #14)
> The visuals seem to follow Eric's spec, but flagging Eric to confirm. 
> As for the title, this should be "Add to Homescreen" according to the spec.

Hey Francis - We've actually gotten feedback that we should display "Add to home screen" everywhere from l10n/copy folks. In fact, I suppose "homescreen" isn't a real word and we were requested to use "home screen" in all places instead, specifically lower cased and two words. If you do find yourself in the spec again soon - can you update these references?
Flags: needinfo?(fdjabri)
(Assignee)

Comment 16

4 years ago
But what should we show for editing bookmarks? https://bug1053758.bugzilla.mozilla.org/attachment.cgi?id=8482238 "Edit in home screen"?
(Assignee)

Updated

4 years ago
Attachment #8482236 - Attachment is obsolete: true
Attachment #8482236 - Flags: ui-review?(epang)
(Assignee)

Updated

4 years ago
Attachment #8482237 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8482238 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8482239 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
(Assignee)

Comment 17

4 years ago
Created attachment 8483294 [details]
Add to home screen
Attachment #8483294 - Flags: ui-review?(epang)
(Assignee)

Comment 18

4 years ago
Created attachment 8483295 [details]
Edit bookmark
Attachment #8483295 - Flags: ui-review?(epang)
(Assignee)

Comment 19

4 years ago
Comment on attachment 8482242 [details]
Go to PR

Fixed test Kevin
Attachment #8482242 - Flags: review?(kgrandon)
(Assignee)

Comment 20

4 years ago
"Rename bookmark", "Rename in home screen",... I need this info before merging, thx

(In reply to Cristian Rodriguez (:crdlc) from comment #16)
> But what should we show for editing bookmarks?
> https://bug1053758.bugzilla.mozilla.org/attachment.cgi?id=8482238 "Edit in
> home screen"?
(Reporter)

Updated

4 years ago
Keywords: polish
I think keeping either "Edit link" or "Edit bookmark" would make the most sense here.
Comment on attachment 8482242 [details]
Go to PR

Let's make sure we hear from UX on the text before landing - thanks!
Attachment #8482242 - Flags: review?(kgrandon) → review+
Comment on attachment 8483295 [details]
Edit bookmark

Looking at the screen shots this looks good to me!  I think 'Edit bookmark'. Francis do you agree?
Attachment #8483295 - Flags: ui-review?(epang) → ui-review+
Flags: needinfo?(epang)
Comment on attachment 8483294 [details]
Add to home screen

Visually looks to spec based on the screen shot.  When flashing the patch I wasn't able to edit the input field and none of the buttons worked, but I'm guessing it's probably my problem.  Thanks!
Attachment #8483294 - Flags: ui-review?(epang) → ui-review+

Updated

4 years ago
Blocks: 1055065

Updated

4 years ago
No longer blocks: 945259
(Assignee)

Updated

4 years ago
Keywords: late-l10n
Whiteboard: [systemsfe]
(Assignee)

Comment 25

4 years ago
I have changed the string to "Edit bookmark" in the patch
(Assignee)

Comment 26

4 years ago
Upps I merged without Francis' feedback :( Sorry, I didn't realize. Francis, if you are ok with "Edit bookmark" would be perfect, in another case, I could revert this patch and address your suggest for the string.

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/c26c8a237bc665a9a7e55dd248ab8734fd7d9959
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Sorry but this needs to be fixed (up to you if you prefer to backout or fix it in a follow-up).
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Changing_existing_strings

Changing case is acceptable, changing strings like this no.

-add-bookmark-header=Add link
-edit-bookmark-header=Edit link
+add-bookmark-header=Add to home screen
+edit-bookmark-header=Edit bookmark
Flags: needinfo?(crdlc)
Sorry, I left my R+ before this change and didn't catch it. I think Cristian might be gone for the day, so let's backout for now and Cristian can re-land after updating the keys.

Reverted: https://github.com/mozilla-b2g/gaia/commit/6eb0f8fee8d14c0868ad9d4b030dcf8bf90f0141
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Cristian Rodriguez (:crdlc) from comment #26)
> Upps I merged without Francis' feedback :( Sorry, I didn't realize. Francis,
> if you are ok with "Edit bookmark" would be perfect, in another case, I
> could revert this patch and address your suggest for the string.
> 
> Merged in master:
> 
> https://github.com/mozilla-b2g/gaia/commit/
> c26c8a237bc665a9a7e55dd248ab8734fd7d9959

Hi Cristian, Edit bookmark is fine with me.
Flags: needinfo?(fdjabri)
Cristian - To clarify, I think you only need to update the l10n keys for the strings and how they're consumed here before landing. They just need to be changed so our l10n teams know they've been updated and can re-translate them. Thanks!
(Assignee)

Comment 31

4 years ago
working on it
Flags: needinfo?(crdlc)
(Assignee)

Comment 32

4 years ago
Merged in master:

https://github.com/mozilla-b2g/gaia/commit/f818d4f9a6b7f5dd14ea7457220ea5764ff49a5a
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 33

4 years ago
Comment on attachment 8482242 [details]
Go to PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: users can see how the bookmark will be on the home screen because the icon is displayed while they are bookmarking so it does happier to end users thought
[Testing completed]: Added unit tests
[Risk to taking this patch] (and alternatives if risky): no alternatives
[String changes made]: yes
Attachment #8482242 - Flags: approval-gaia-v2.1?
Attachment #8482242 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
v2.1: https://github.com/mozilla-b2g/gaia/commit/1d5474a788240919b5d15a7f9e0c56d0dd051072
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
Target Milestone: --- → 2.1 S4 (12sep)
You need to log in before you can comment on or make changes to this bug.