1216432 - Keys on Emoji swipe panel does not layout correctly in landscape mode

Status: RESOLVED FIXED

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

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

STR:

1. Open a web page that is not orientation locked
2. Focus on rocket bar
3. Enable Emoji layout
4. Put the phone on landscape

Expected:

1. Emoji swipable panel works as expected

Actual:

1. Panel is rendered incorrectly and swipe does not work.

Raze, unfortunately my team member previously worked on this feature did not complete the work with quality. I think we should not land bug 993899 before this is fixed since this is quite serious. Do you agree?

Comment 1

[Reza Akhavan [:jedireza]]

Tim, I'm using the latest version of the the PR on bug 993899 (the PR is up to date with master as of a few minutes ago). And and I wasn't able to reproduce the issue. I noticed the spacing of the keys could be better, but it's not something I'd block on. I tested this with a Flame and an Aries.

Comment 2

[Reza Akhavan [:jedireza]]

Attachment: flame horizontal emoji keyboard

Comment 3

[Reza Akhavan [:jedireza]]

Attachment: aries horizontal emoji keyboard

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Understood and agreed, let's not block on bug 993899.

(not removing the bug from "Blocks" because we don't have a follow-up tracker yet)

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ray, would you be able to work on this after Gij tests?

First, by inspecting http://timdream.org/gaia-keyboard-demo/#emoji I can see .emoji { min-width: } should have been set to 16.66vw (1/6 of the width). This fix the position of the keys on the swipe panel.

Secondly, the swipe-section divs have |transform: transitionX()| values set by JavaScript, which should have been recalculated when resize took place. Please look at ViewManager and investigate how to properly dispatch the event.

Alternatively, swipe can be re-implemented with actual scrolling + scroll-snap (https://devdocs.io/css/scroll-snap-type). I am also happy if you would like to investigate this route (and remove our old JavaScript swiping).

Thanks!

Comment 6

[Ray Lin[:ralin]]

I'm willing to take this bug. I will study related techniques after Gij tests, then take a look at this bug.

Comment 7

[Ray Lin[:ralin]]

Tim,

Position is fixed, but I met a problem about scroll-snap:
If changed to scroll-snap, scroll bar appears while scrolling and it plays a very similar role as indicator does. 
scroll bar can be used to replace original indicator, however, scroll bar could not show all the time for system limitation. It will vanish after a second idle.
The other way is to keep original indicator, and hide scroll bar while scrolling. It's possible to do that, but in improper way.

I've tried scroll-snap, and it swiped smoother than previous work. Therefore, I'd like to re-implement swiping with scroll-snap, but I need to fix the problem to deliver consistent user experience.

Could you give me some advice about this problem? or it can also be solved by just fixing previous work?

Thank you very much!

Comment 8

[Ray Lin[:ralin]]

After discussion with UX, we think that scroll bar could not take over the role of indicator, and the performance issue just slightly affect on user experience. 

I would get around to fix resize problem.

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] raylin:1216432-emoji-landscape-layout-r1 > mozilla-b2g:master

Comment 10

[Ray Lin[:ralin]]

Comment on attachment 8695085 [details] [review]
[gaia] raylin:1216432-emoji-landscape-layout-r1 > mozilla-b2g:master

I override inherited resize function and propagate the event to swipeable panel. Now emoji panel updates sections' position(translateX) when resize according to ViewManager.

Thanks,

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8695085 [details] [review]
[gaia] raylin:1216432-emoji-landscape-layout-r1 > mozilla-b2g:master

Nice. Please address this issue:

https://github.com/mozilla-b2g/gaia/pull/33490/files#r46528421

Also please see if it's possible to assert rotate functions with unit tests.

Comment 12

[Ray Lin[:ralin]]

PR updated 

SwipeDetector is disabled while rotating, and without users' interference, panel renders correctly and stably.
for unit test, is it ok to file another bug?

Comment 13

[Ray Lin[:ralin]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #11)
> Comment on attachment 8695085 [details] [review]
> [gaia] raylin:1216432-emoji-landscape-layout-r1 > mozilla-b2g:master
> 
> Nice. Please address this issue:
> 
> https://github.com/mozilla-b2g/gaia/pull/33490/files#r46528421
> 
> Also please see if it's possible to assert rotate functions with unit tests.

PR updated. 

Add rotate(resize function) unit tests. Thanks.

Comment 14

[Ray Lin[:ralin]]

Comment on attachment 8695085 [details] [review]
[gaia] raylin:1216432-emoji-landscape-layout-r1 > mozilla-b2g:master

Hi,

Fix and update PR again. 

With Tim's clarification, this version sets a flag to temporarily pause pan and swipe handlers after resized.


Thanks,

Comment 15

[Ray Lin[:ralin]]

https://github.com/mozilla-b2g/gaia/commit/9a2f7dc71d67a24524d7b829f8d5cd191d9c0ba4