863323 - [Bluetooth] Unable to Pair Screen Updates

Status: RESOLVED WORKSFORME

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

Description

[Eric Pang [:epang] UX]

Attachment: Unable to Pair Screen Updates Mock Up

Hi Pavel,

Can you update the layout of this screen?  The mock up and specs will be attache shortly.  Thanks!

Comment 1

[Eric Pang [:epang] UX]

Attachment: Font Specs

Specs for Fonts

Comment 2

[Eric Pang [:epang] UX]

Attachment: Unable to Pair Screen Updates Mock Up

Updated padding of div lines to 15px (to match revised building blocks)

Comment 3

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: After patch

Comment 4

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia repo

Comment 5

[Eric Pang [:epang] UX]

(In reply to Pavel Ivanov [:ivanovpavel] from comment #3)
> Created attachment 741701 [details]
> After patch

Looks good, thanks for making the update Pavel!

Comment 6

[Sam Joch [:samjoch]]

Comment on attachment 741702 [details]
patch for Gaia repo

This should really be part of the BB, it's a simple alert message.

Comment 7

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

(In reply to Sam Joch [:samjoch] from comment #6)
> Comment on attachment 741702 [details]
> patch for Gaia repo
> 
> This should really be part of the BB, it's a simple alert message.

I agree but I think we need to change some of the logic in system app, right?

Comment 8

[Eric Pang [:epang] UX]

(In reply to Pavel Ivanov [:ivanovpavel] from comment #7)
> (In reply to Sam Joch [:samjoch] from comment #6)
> > Comment on attachment 741702 [details]
> > patch for Gaia repo
> > 
> > This should really be part of the BB, it's a simple alert message.
> 
> I agree but I think we need to change some of the logic in system app, right?

What should we do about this Sam?

Comment 9

[Eric Pang [:epang] UX]

Sergi, we want to implement the Bluetooth pairing screens with the BB, but we're having trouble as Pavel stated in comment 7.  By any chance can you help us with a solution? Thanks!

Comment 10

[Sergi]

(In reply to Eric Pang [:epang] from comment #9)
> Sergi, we want to implement the Bluetooth pairing screens with the BB, but
> we're having trouble as Pavel stated in comment 7.  By any chance can you
> help us with a solution? Thanks!

AFAIK alert screens like the one you're using are system components. My suggestion is to use the "Confirm" BB. The problem is the BB is not yet updated with the 15px/30px padding rule.

I'm adding a new bug so Arnau updates "Confirm" to the last spec.

Comment 11

[Sam Joch [:samjoch]]

I agree w/ Sergi.

Even if it's a system panel, it's using BB as well. (https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/bluetooth.js#L485)

So we should update BB to match the right padding. This should fix this bug automatically.

Eric: Should Pavel updates the BB and let Arnau r+ his patch ?

Comment 12

[Eric Pang [:epang] UX]

(In reply to Sam Joch [:samjoch] from comment #11)
> I agree w/ Sergi.
> 
> Even if it's a system panel, it's using BB as well.
> (https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/bluetooth.
> js#L485)
> 
> So we should update BB to match the right padding. This should fix this bug
> automatically.
> 
> Eric: Should Pavel updates the BB and let Arnau r+ his patch ?

this sounds good to me! Thanks Sam

Comment 13

[Sam Joch [:samjoch]]

Actually i would like being aware of all BB updates. So please CC in related bugs.

Comment 14

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 741702 [details]
patch for Gaia repo

Everything ok!

Comment 15

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Arnau :)
landed on master: https://github.com/mozilla-b2g/gaia/commit/3f21ffb7ea4b574ee03f5cd67314fe17c928ba72

Comment 16

[Eric Pang [:epang] UX]

Naoki, this should be ready for verification! :)

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

This patch converts <div> to <form> and use <button> elements. Sadly buttons elements are the equivalent of a <input type="submit"> not an <input type="button"> so the form is submitted and the system app is reloaded which breaks a lot of things...

Comment 18

[David Flanagan [:djf]]

I've backed out the change: https://github.com/mozilla-b2g/gaia/commit/7f90b27f720784e7c4d4fcd055118c16a44778fe

Comment 19

[Eric Pang [:epang] UX]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #17)
> This patch converts <div> to <form> and use <button> elements. Sadly buttons
> elements are the equivalent of a <input type="submit"> not an <input
> type="button"> so the form is submitted and the system app is reloaded which
> breaks a lot of things...

Thanks Vivien for the notes!

Not ready for verification... Pavel can you look into Vivien's comments?  If extra info is needed please clarify with Vivien or Arnau, thanks!

Comment 20

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Vivien and David,
sorry for the inconvenience ... I will try to fix this one and take a note for the [BB]

Comment 21

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Arnau,
can we change the rules here:
https://github.com/mozilla-b2g/gaia/blob/master/shared/style/confirm.css

form[role="dialog"][data-type="confirm"],
div[role="dialog"][data-type="confirm"]

or maybe it's a better idea to use class name:

form[role="dialog"][data-type="confirm"],
.modal[role="dialog"][data-type="confirm"]

What do you think?

Comment 22

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Pavel,
Dialogs will always have a button to close or to trigger an action, so make sense using form.
Why do you need to use div instead of form?
Although for me it would make more sense just having:
[role="dialog"][data-type="confirm"]
than duplicating selectors.
Thanks

Comment 23

[Eric Pang [:epang] UX]

(In reply to arnau from comment #22)
> Pavel,
> Dialogs will always have a button to close or to trigger an action, so make
> sense using form.
> Why do you need to use div instead of form?
> Although for me it would make more sense just having:
> [role="dialog"][data-type="confirm"]
> than duplicating selectors.
> Thanks

Pavel, with the info from Pavel are you able to proceed with this bug?

Comment 24

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

not realy,
Arnau you can see Comment 17 from Vivien.

Comment 25

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

What if we replace form[role="dialog"][data-type="confirm"] for [role="dialog"][data-type="confirm"] without the form in the BB?

Comment 26

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

we can do this ... it's not very fast selector but will help us right now. I will open a bug for this if you don't mind?

Comment 27

[Ismael Gonzalez [:basiclines]]

Can we simply add some basic js to prevent <form> submission?
myForm.onsubmit = function(e){ e.preventDefault(); };

I mean, we are using this "confirm" in many applications and there wasn't any problem about using it in this way.

Anyway, if you are unable to use this kind of js behaviour for any reason, i suggest to change the selector to [data-type="confirm"] which is equally expensive than the current one.

Hope it helps!

Comment 28

[Eric Pang [:epang] UX]

(In reply to Ismael Gonzalez [:basiclines] from comment #27)
> Can we simply add some basic js to prevent <form> submission?
> myForm.onsubmit = function(e){ e.preventDefault(); };
> 
> I mean, we are using this "confirm" in many applications and there wasn't
> any problem about using it in this way.
> 
> Anyway, if you are unable to use this kind of js behaviour for any reason, i
> suggest to change the selector to [data-type="confirm"] which is equally
> expensive than the current one.
> 
> Hope it helps!

Hey Pavel, have you seen Ismael's comment to help with this bug?

Comment 29

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

I saw the comment now ... but I think this bug is fixed by:
https://github.com/mozilla-b2g/gaia/commit/d1463bc3cfe39e9c04a4de0c3f9e9908988834b4

Comment 30

[Eric Pang [:epang] UX]

Great, thanks Pavel!