Incall keypad layout needs adjustment

RESOLVED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Dialer
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: drs, Assigned: paco)

Tracking

unspecified
2.1 S6 (10oct)

Firefox Tracking Flags

(b2g-v2.2 fixed)

Details

(Whiteboard: [planned-sprint])

Attachments

(7 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8482489 [details]
2014-08-19-13-46-05.png
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1059610
(Reporter)

Comment 2

4 years ago
We need a spec for this before it's actionable.
Flags: needinfo?(chuang)

Comment 3

4 years ago
Created attachment 8482492 [details]
layout_0902.jpg

Hi,
I attached the 'In call keypad layout'

Since the 320x480 looks no problem, I only have 480x854 in the spec.
Basically, I would like to have the overlay fill the whole screen not showing the bg(contact photo).

Please see the attachment and let me know if there's any question.
Thank you!
Flags: needinfo?(chuang)
(Reporter)

Updated

4 years ago
Whiteboard: [planned-sprint]
(Reporter)

Updated

4 years ago
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
(Assignee)

Updated

4 years ago
Assignee: nobody → pacorampas
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
Created attachment 8489336 [details] [review]
patch in github

Hi Doug,

I have done this first test of how fix it. With this patch I want to fill the gap between keypad and phone numbers (#calls) painting the background of #calls. If we want continue with this approach we need to make some changes for painting better and easier the numbers wrappers.

Also, we can bet for other approach. We can put a extra class for differentiate if there is a single-call or there are two calls when is opened the keypad. Then, we can play with the height of keypad for full fill.

What do you think? What is the best bet?

Tanks.
Attachment #8489336 - Flags: feedback?(drs+bugzilla)
(Reporter)

Comment 5

4 years ago
Comment on attachment 8489336 [details] [review]
patch in github

Paco, thanks for this patch. For starters, I think this needs more input from Carol. We can tackle the technical decisions once we iron out the VD.

In particular, trying this patch made me realize that the transition to clipping the background with the keypad isn't well-defined. I took a video to show this which can be found here:
http://youtu.be/4s82vJ6TaBM

Two possibilities I can think of:
1) We add an animation to the contact info section, expanding it downwards to meet the keypad.
2) We extend the keypad and make it as large as is needed to clip the background once it slides in.

Carol, what do you think here? Do you have an alternative suggestion?

I'd also like to ask, what is the rationale for this change? I'd like to hear your thoughts on the deficiencies of the current design.
Attachment #8489336 - Flags: feedback?(drs+bugzilla)
Flags: needinfo?(chuang)

Comment 6

4 years ago
Hi Doug and Paco,
From Visual's point of view, I don’t want to leave the gap because we can’t be sure about what’ will be in the background. Since it will be the contact photo, It might be a person’s huge face. The rectangle space might just showed his eyes or part of the nose. So it would be visually distracting with the gap.


I’ve watched the video of calling up the keypad. I was hoping the mask would be long enough to fill up the entire screen. I didn’t expect another mask covering the rectangle space ‘after’ the keypad got to the spot. So It’s a bit different from what I was thinking.
Thanks!
Flags: needinfo?(chuang) → needinfo?(pacorampas)
(Assignee)

Comment 7

4 years ago
I think do you mean the second choice that Doug has explained. I bet for this option too.
Flags: needinfo?(pacorampas)

Comment 8

4 years ago
Hi Paco,
Yes, I think I'm talking about the same thing as the second option that Doug mentioned in comment 5. Thanks :))
(Reporter)

Comment 9

4 years ago
Ok, thanks Carol.

Paco, my app manager is acting up so it's hard for me to fiddle with CSS. I think the best way to fix this is to make the keypad container flex so that it expands to fill the screen, minus the calls section. We can then lock the keypad itself to the bottom of the keyboard container.

I quickly experimented with this by adding the following:

body.showKeypad #keyboard-container {
  -moz-box-flex: 1;
  flex: 1 1 0%;
}

This expanded the container, but the background wasn't completely clipped, and the keypad was at the top of the container instead of the bottom. So you'll have to fiddle with it a bit. Your results may vary here, but I think that using flex is probably the way to go.
(Assignee)

Comment 10

4 years ago
> So you'll have to fiddle with it a bit. Your results may vary here, but I think
> that using flex is probably the way to go.

I think using flex is the best option, but we need to do a lot of changes.

Firstly, we need change all the call screen layout for doing it with flex display (not absolute positions).
Once we have this, we will get a layout where "the views wrappers" will get their height in consequence of the height of their sons.

Then, the last thing is change the way of putting the background, I think is better as background of body.
This is a deep change for only have a bigger keypad.

If we will do it I think we should split all the work in different bugs.
Can we do a prototype where we just hard code the new height of the keypad to see if the transition works?

I'm concerned that with the call duration that has to disappear, this won't look nice.
(Assignee)

Comment 12

4 years ago
> I'm concerned that with the call duration that has to disappear, this won't
> look nice.

Here a video demo. For me  the transition works fine.
https://www.youtube.com/watch?v=ye5jLLmWU_U&feature=youtu.be
(Assignee)

Comment 13

4 years ago
Hello, 
What is the decision here? Do the demo transitions look nice?
Flags: needinfo?(drs+bugzilla)
Flags: needinfo?(anthony)
(Reporter)

Comment 14

4 years ago
(In reply to Paco Rampas [:paco] from comment #10)
> I think using flex is the best option, but we need to do a lot of changes.
> 
> Firstly, we need change all the call screen layout for doing it with flex
> display (not absolute positions).
> Once we have this, we will get a layout where "the views wrappers" will get
> their height in consequence of the height of their sons.
> 
> Then, the last thing is change the way of putting the background, I think is
> better as background of body.
> This is a deep change for only have a bigger keypad.
> 
> If we will do it I think we should split all the work in different bugs.

These changes are definitely out of the scope of this bug. For now, it would be better to just apply classes to the keypad to get the right size depending on the situation.

(In reply to Paco Rampas [:paco] from comment #13)
> Hello, 
> What is the decision here? Do the demo transitions look nice?

I think it looks fine.
Flags: needinfo?(drs+bugzilla)
(Assignee)

Comment 15

4 years ago
Created attachment 8493628 [details] [review]
patch in github
Attachment #8489336 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8493628 - Flags: review?(drs+bugzilla)
(Reporter)

Comment 16

4 years ago
Created attachment 8493872 [details]
Screenshots of attachment 8493628 [details] [review]

Paco, there are a few issues with this patch.

In the screenshot on the left, you can see that the keypad overlaps the call indicators. To get this, I did the following:

1. Start a call.
2. Open the keypad.
3. Have another device call the test device, answer it.
4. Open the keypad again.

In the screenshot on the right, you can see the "via SIM1" indicator through the transparent keypad. There's also a small line where the keypad isn't fully covering the background.

I also noticed that the duration indicator doesn't fade away smoothly but just vanishes, and you can see this transition behind the keypad. I find it rather jarring and would prefer to transition the opacity. Though we should test this and see how performant it is.

Finally, this will need some unit tests.
(Reporter)

Comment 17

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

Also clearing Anthony's needinfo as I don't think it's needed anymore. Feel free to comment/reinstate it.
Attachment #8493628 - Flags: review?(drs+bugzilla)
Flags: needinfo?(anthony)
(Assignee)

Comment 18

4 years ago
Created attachment 8495152 [details]
peak_keypad.png
(Assignee)

Comment 19

4 years ago
> In the screenshot on the left, you can see that the keypad overlaps the call
> indicators.
I have uploaded the patch and this is solved. If you want you can check it.

> In the screenshot on the right, you can see the "via SIM1" indicator through
> the transparent keypad. 
I need a dual SIM phone for reproduce this issue. I am trying to get one. 

>There's also a small line where the keypad isn't fully covering the background.
I don't reproduce this issue. What phone reproduce this issue? I'm using peek for this attachment 8495152 [details].

> I also noticed that the duration indicator doesn't fade away smoothly but
> just vanishes, and you can see this transition behind the keypad. 
Done it.
(Assignee)

Comment 20

4 years ago
I said "peek", but I wanted say "peak" ;)
(Assignee)

Updated

4 years ago
Attachment #8495152 - Flags: ui-review?(cawang)
(Assignee)

Comment 21

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

I have found why the there was a line between keypad and the calls info with a single call. The issue occurs when you have the contact saved and, inconsequence, it is showing number and name (not only number).
Then, now, it is all working fine. Only I should update the tests.
Attachment #8493628 - Flags: feedback?(drs+bugzilla)
(Assignee)

Comment 22

4 years ago
Created attachment 8495233 [details]
demo video
Attachment #8495233 - Flags: ui-review?(cawang)
(Reporter)

Updated

4 years ago
Attachment #8495152 - Flags: ui-review?(cawang) → ui-review?(chuang)
(Reporter)

Comment 23

4 years ago
Comment on attachment 8495233 [details]
demo video

I find that the way the keypad disappears is rather jarring as there's no transition. But that's out of the scope of this bug. We can file a followup for that if others agree.
Attachment #8495233 - Flags: ui-review?(cawang) → ui-review?(chuang)
(Reporter)

Comment 24

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

I generally like this approach and the CSS looks reasonable. From a quick on-device test, this worked well and I didn't see any problems.

I'm reluctantly setting feedback- because I think we should combine the two .single-line classes. Instead of setting it on both the `body` and `#calls` elements, we should just apply it to `body` and make the `#calls` one use that instead.
Attachment #8493628 - Flags: feedback?(drs+bugzilla) → feedback-
(Assignee)

Comment 25

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

I have done the latest changes. I removed the #calls.single-line, now we have only the body.single-line. I have changed all selectors for that.

Also, I have written the a new tests.

Could you check it?

Thanks :D
Attachment #8493628 - Flags: feedback- → feedback?(drs+bugzilla)
(Reporter)

Comment 26

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

This looks pretty good. I left a couple of nits on the PR. You can take this into review after they're fixed.
Attachment #8493628 - Flags: feedback?(drs+bugzilla) → feedback+
Comment on attachment 8495233 [details]
demo video

I think it looks fine. Thanks, Paco!
Attachment #8495233 - Flags: ui-review?(chuang) → ui-review+
Comment on attachment 8495152 [details]
peak_keypad.png

I'm not sure why the dialing number is fade away. If the number gets too long, it should be "..." before it touched the icon on the right.
Attachment #8495152 - Flags: ui-review?(chuang) → ui-review-
(Reporter)

Comment 29

4 years ago
Comment on attachment 8495152 [details]
peak_keypad.png

(In reply to Carol Huang [:Carol] from comment #28)
> Comment on attachment 8495152 [details]
> peak_keypad.png
> 
> I'm not sure why the dialing number is fade away. If the number gets too
> long, it should be "..." before it touched the icon on the right.

I think that was because Paco didn't want to post his phone numbers online, so he edited them out. I can confirm that this isn't how it actually looks as I've tried this patch. Is this screenshot acceptable, if we ignore the fading?
Attachment #8495152 - Flags: ui-review- → ui-review?(chuang)
Comment on attachment 8495152 [details]
peak_keypad.png

Hey Doug,
Thanks for explanation. the screen looks good!
Attachment #8495152 - Flags: ui-review?(chuang) → ui-review+
(Assignee)

Comment 31

4 years ago
> Paco didn't want to post his phone numbers online,
Doug is right. Because one of both numbers is my personal number. But, some times I forgot edit the shots, then I am sure that my number is online hehehe.

Thanks carol for R+ :)
(Assignee)

Comment 32

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

Hi Doug, 

I have left a comment on github about one of your nits.
Attachment #8493628 - Flags: review?(drs+bugzilla)
Target Milestone: 2.1 S5 (26sep) → 2.1 S6 (10oct)
(Reporter)

Comment 33

4 years ago
Comment on attachment 8493628 [details] [review]
patch in github

Thanks, looks good. I left one nit on the PR.
Attachment #8493628 - Flags: review?(drs+bugzilla) → review+
(Assignee)

Comment 34

4 years ago
Created attachment 8498680 [details] [review]
patch in github reopened

Hi Doug, 

The last PR was closed, therefore I have reopened another PR and I have asked to you for review, but is the same code that the patch that have R+.
Attachment #8498680 - Flags: review?(drs+bugzilla)
(Reporter)

Updated

4 years ago
Attachment #8498680 - Flags: review?(drs+bugzilla) → review+
(Reporter)

Comment 35

4 years ago
https://github.com/mozilla-b2g/gaia/commit/27cbf551e1e9f699b64abc381184515d6483ec43
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-b2g-v2.2: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.