Closed Bug 1061398 Opened 10 years ago Closed 10 years ago

Incall keypad layout needs adjustment

Categories

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

defect
Not set
normal

Tracking

(b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S6 (10oct)
Tracking Status
b2g-v2.2 --- fixed

People

(Reporter: drs, Assigned: paco)

References

Details

(Whiteboard: [planned-sprint])

Attachments

(7 files, 1 obsolete file)

Attached image 2014-08-19-13-46-05.png (deleted) —
No description provided.
We need a spec for this before it's actionable.
Flags: needinfo?(chuang)
Attached image layout_0902.jpg (deleted) —
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)
Whiteboard: [planned-sprint]
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
Assignee: nobody → pacorampas
Status: NEW → ASSIGNED
Attached file patch in github (obsolete) (deleted) —
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)
Comment on attachment 8489336 [details] 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)
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)
I think do you mean the second choice that Doug has explained. I bet for this option too.
Flags: needinfo?(pacorampas)
Hi Paco, Yes, I think I'm talking about the same thing as the second option that Doug mentioned in comment 5. Thanks :))
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.
> 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.
> 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
Hello, What is the decision here? Do the demo transitions look nice?
Flags: needinfo?(drs+bugzilla)
Flags: needinfo?(anthony)
(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)
Attached file patch in github (deleted) —
Attachment #8489336 - Attachment is obsolete: true
Attachment #8493628 - Flags: review?(drs+bugzilla)
Attached image Screenshots of attachment 8493628 (deleted) —
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.
Comment on attachment 8493628 [details] 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)
Attached image peak_keypad.png (deleted) —
> 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.
I said "peek", but I wanted say "peak" ;)
Attachment #8495152 - Flags: ui-review?(cawang)
Comment on attachment 8493628 [details] 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)
Attached file demo video (deleted) —
Attachment #8495233 - Flags: ui-review?(cawang)
Attachment #8495152 - Flags: ui-review?(cawang) → ui-review?(chuang)
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)
Comment on attachment 8493628 [details] 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-
Comment on attachment 8493628 [details] 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)
Comment on attachment 8493628 [details] 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-
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+
> 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+ :)
Comment on attachment 8493628 [details] 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)
Comment on attachment 8493628 [details] patch in github Thanks, looks good. I left one nit on the PR.
Attachment #8493628 - Flags: review?(drs+bugzilla) → review+
Attached file patch in github reopened (deleted) —
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)
Attachment #8498680 - Flags: review?(drs+bugzilla) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: