828851 - Settings view should have a done button on the right instead of a cross closing icon on the left

Status: VERIFIED FIXED

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

Comment 1

[sergey.rossokha]

I have fixed this, pull request link: https://github.com/mozilla-b2g/gaia/pull/7777

Comment 2

[Alex Keybl [:akeybl]]

Can you provide a screenshot?

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking, please provide screenshot and approval nomination request for uplift consideration.

Comment 4

[Jose Manuel Cantera]

(In reply to sergey.rossokha from comment #1)
> I have fixed this, pull request link:
> https://github.com/mozilla-b2g/gaia/pull/7777

Sergey please could you ask me for a review? You can attach your PR similarly as it has been done for instance in bug 835555 and then put the r? flag over it

thanks!

Comment 5

[sergey.rossokha]

Hi Joe, how I can do this? Sorry I'm new in BZ :(

Comment 6

[sergey.rossokha]

Hi Jose, how I can do this? Sorry I'm new in BZ :(

Comment 7

[sergey.rossokha]

Attachment: Contact settings page with "Done" button

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):

Comment 8

[sergey.rossokha]

Attachment: Pointer to GH 7777 PR

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):

Comment 9

[Jose Manuel Cantera]

Comment on attachment 710973 [details]
Contact settings page with "Done" button

Sergei,

Before merging this, Please we need you to change the commit message to be equal to the bug number + description as it happens with other commits. If you don't know how to do do that please ask me by e-mail or IRC 'joseCantera'

thanks!

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 710983 [details]
Pointer to GH 7777 PR

Assuming the r+ was meant for this code change, approving for v1.0.1.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 710973 [details]
Contact settings page with "Done" button

Clearing the approval flag on this screenshot.

Comment 12

[Cristian Rodriguez (:crdlc)]

Attachment: The same but with correct commit message

Comment 13

[Cristian Rodriguez (:crdlc)]

https://github.com/mozilla-b2g/gaia/commit/a22bac2a4e4542a01654b9d3b7ae1bd50afd7199

Comment 14

[Jose Manuel Cantera]

Comment on attachment 712854 [details]
The same but with correct commit message

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: UX global consistency will suffer
Testing completed: yes
Risk to taking this patch (and alternatives if risky): very low, only presentation changed

Comment 15

[Sergi]

Attachment: Contact settings page with "Done" button

I've updated the reference screenshot because it was using some outdated visual elements

Comment 16

[Sergi]

It's important to land this patch asap because it has a huge impact in the app experience. As far as we are not saving the state when the user makes a change, but we are applying it immediately, it makes no sense to have an "X" and a "Done" button in the header because both of them do the same thing. The recommendation is to leave the "Done" button and remove the "X".

Comment 17

[Jose Manuel Cantera]

patch landed on master today and hopefully will do so in train once we get an approval

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 72d348085bf20a68cacc5825374ef23a99d9522a

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1

Comment 20

[isabelrios]

Verified with 02/18 build: Gecko-0ecc076.Gaia-4164424