841439 - [Cost Control] Cost Control app should use system dialogs instead of custom views implemented in the app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[Michał Budzyński (:michalbe)]

I don't understand why we need to implement something that is already implemented. 
The only differences we can find in those dialogs are:

- cancel button in select-value dialog
- different colors of buttons
- different labels on buttons

Its not necessary to introduce new elements just for that - default value that is selected after select-value dialog is launched is the last value selected, so clicking 'ok' without selecting the new option works like cancel.
Also - we can live with 'Cancel' & blue 'OK' buttons instead of Cancel and red 'Yes' when reseting data usage - our main system hard reset form settings use this dialog, and it's way more important than just resting the data (see attachments).

Comment 1

[Michał Budzyński (:michalbe)]

Attachment: system reset (from settings) with system dialog

Comment 2

[Michał Budzyński (:michalbe)]

Attachment: CC data usage reset (with custom dialog)

Comment 3

[Michał Budzyński (:michalbe)]

Attachment: system select-value dialog with one button

Comment 4

[Michał Budzyński (:michalbe)]

Attachment: CC custon select-value dialog (with two buttons)

Comment 5

[Michał Budzyński (:michalbe)]

Another thing - spec of the dialogs have been changed and now all the system dialogs fade in & out instead of sliding from the bottom (Bug #841027). Cost control still use sliding.
Changes like this would be easier in the future if we will use system dialogs like all the other applications.

Comment 6

[Salvador de la Puente González [:salva]]

(In reply to Michal Budzynski (:michalbe) from comment #0)
> I don't understand why we need to implement something that is already
> implemented. 
> The only differences we can find in those dialogs are:
> 
> - cancel button in select-value dialog
> - different colors of buttons
> - different labels on buttons
> 
> Its not necessary to introduce new elements just for that - default value
> that is selected after select-value dialog is launched is the last value
> selected, so clicking 'ok' without selecting the new option works like
> cancel.

Well, if you are choosing from 31 different elements, I would want to have a way to cancel my selection without remembering the former one.

> Also - we can live with 'Cancel' & blue 'OK' buttons instead of Cancel and
> red 'Yes' when reseting data usage - our main system hard reset form
> settings use this dialog, and it's way more important than just resting the
> data (see attachments).

Here I think no and I dare to say our implementation provide more value to the user than the one in settings because we are highlighting that 'Reset' is an important action.

Let's ask Marco about this.

Comment 7

[Salvador de la Puente González [:salva]]

Anyway Michal, I think you're mostly right so I take the bug in order to change it because it affects auto_settings.js module. ;)

Comment 8

[marcoc]

(In reply to Salvador de la Puente González [:salva] from comment #6)
> (In reply to Michal Budzynski (:michalbe) from comment #0)
> > I don't understand why we need to implement something that is already
> > implemented. 
> > The only differences we can find in those dialogs are:
> > 
> > - cancel button in select-value dialog
> > - different colors of buttons
> > - different labels on buttons
> > 
> > Its not necessary to introduce new elements just for that - default value
> > that is selected after select-value dialog is launched is the last value
> > selected, so clicking 'ok' without selecting the new option works like
> > cancel.
> 
> Well, if you are choosing from 31 different elements, I would want to have a
> way to cancel my selection without remembering the former one.

With value selectors: 
I agree with Michal here that consistency matters. Hence it is a question of following the latest definition of Building Blocks for this scenario. One button only that is a blue OK. 

> 
> > Also - we can live with 'Cancel' & blue 'OK' buttons instead of Cancel and
> > red 'Yes' when reseting data usage - our main system hard reset form
> > settings use this dialog, and it's way more important than just resting the
> > data (see attachments).
> 
> Here I think no and I dare to say our implementation provide more value to
> the user than the one in settings because we are highlighting that 'Reset'
> is an important action.

With confirmation dialogs (in the case of a data reset that has an important consequence):
I agree with Salva that it should be according to Building Blocks with a grey Cancel button and red Reset button. Reset should be re-iterated in the button given the consequence of action and users many times don't read the full message above. 

The main system reset is not a good example to follow as it should also have reset in red given the very large consequence this action has, and the hence need to differentiate this confirmation from other less impactfull blue confirmations.

> 
> Let's ask Marco about this.

Comment 9

[Salvador de la Puente González [:salva]]

Then, I will move to system selects and general dialogs but leaving our reset dialog.

Comment 10

[Alex Keybl [:akeybl]]

(In reply to Salvador de la Puente González [:salva] from comment #9)
> Then, I will move to system selects and general dialogs but leaving our
> reset dialog.

Why should we consider blocking shira on this bug?

Comment 11

[Salvador de la Puente González [:salva]]

Ups, I want to mean tef? Sorry.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

While this seems like a good thing to do, we are in the end-game and this cosmetic change would not block a release.

Comment 13

[Salvador de la Puente González [:salva]]

Attachment: Custom select dialogs replaced by standard <selects> and custom warnings removed in favor of system alerts

Some CSS and additional markup required to stylish select boxes as pointed by building blocks.

Comment 14

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 715980 [details]
Custom select dialogs replaced by standard <selects> and custom warnings removed in favor of system alerts

Tried on the phone, looking good, mainly markup and css changes :)

Comment 15

[Salvador de la Puente González [:salva]]

Master: 8b955ec03bb86187e2a21c5d24c0ff06e35324cb

Comment 16

[Salvador de la Puente González [:salva]]

Comment on attachment 715980 [details]
Custom select dialogs replaced by standard <selects> and custom warnings removed in favor of system alerts

As markup is reduces, a little increase of performance is achieved.

Bug caused by (feature/regressing bug #): this one
User impact if declined: moderate
Testing completed: yes
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch:

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 715980 [details]
Custom select dialogs replaced by standard <selects> and custom warnings removed in favor of system alerts

Approved for v1-train uplift.

Comment 18

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

Uplifted commit 8b955ec03bb86187e2a21c5d24c0ff06e35324cb as:
v1-train: c2d63f7aac31bc35dec9c22224fe61bd5ee53853

Comment 20

[Salvador de la Puente González [:salva]]

This bug is causing a moderate usability problem. See bug 825206 for details. Renominating as tef?

Comment 21

[Daniel Coloma:dcoloma]

I think it is a bit late to add this to 1.0.1