Closed Bug 1032768 Opened 10 years ago Closed 10 years ago

Change software home button location in landscape to minimize impact on screen size.

Categories

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

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: markus, Unassigned)

References

Details

Attachments

(2 files, 1 obsolete file)

This bug is to suggest a change of the software home button location in landscape mode to maximize available screen area for apps. The attached patch adds the following features: * Soft home button is located at the bottom of the screen in portrait and to the right in landscape. * The button animates in/out when it's shown/hidden. * It's hidden when apps enter fullscreen (e.g. video player), but shown when the user taps the screen. If there are no further action from the user, the button will automatically be hidden again after a timeout. * The home button bar follows the transparency state of the system bar. Known limitations in this implementation: * In landscape the soft home button is partly covered by the (invisible) swipe detection area. * Some z-index problems when swiping between apps in landscape.
Attachment #8448676 - Flags: ui-review?(padamczyk)
Attachment #8448676 - Flags: review?(timdream)
Attachment #8448676 - Flags: review?(timdream) → review?(alive)
Comment on attachment 8448676 [details] [diff] [review] Change-software-home-button-location-in-landscape.patch Review of attachment 8448676 [details] [diff] [review]: ----------------------------------------------------------------- The issue you mentioned is important to me so I won't review until UX confirmed. Rob, please let me know what do you think about this: the patch render the software home button in right hand side when landscape but it's conflicting with edge gesture. ::: apps/system/js/layout_manager.js @@ +65,4 @@ > * @memberOf LayoutManager > */ > get width() { > + return window.innerWidth - softwareButtonManager.width; you will need a test in layout_manager_test.js ::: apps/system/style/software_button_manager/software_button_manager.css @@ +16,5 @@ > +#screen.on-homescreen > #software-buttons.semiTransparent { > + background-color: rgba(0, 0, 0, 0.2); > +} > + > +#screen.sbTransition > #windows > .appWindow { put window style in window.css
Attachment #8448676 - Flags: review?(alive) → ui-review?(rmacdonald)
Attachment #8448676 - Flags: ui-review?(padamczyk)
Comment on attachment 8448676 [details] [diff] [review] Change-software-home-button-location-in-landscape.patch Hi Markus... I downloaded the patch but I received a lot of errors. Talking to one of the engineers he said the code may be out of date. Please NI me if there is an updated patch UX should be reviewing. Also, fyi, within UX we're more used to downloading patches from github. Not sure if this is an option for you but, if it is, it would definitely speed up UX reviews. :) Thanks! Rob
Attachment #8448676 - Flags: ui-review?(rmacdonald) → ui-review-
Comment on attachment 8448676 [details] [diff] [review] Change-software-home-button-location-in-landscape.patch Review of attachment 8448676 [details] [diff] [review]: ----------------------------------------------------------------- Hey Markus, this is really promising! I know there's still some ongoing UX work but maybe we can start tackling the edge gesture issue in the meantime. There's already code in the edge swipe detector to differentiate between a swipe and a tap/long tap so we can probably re-use that by - making sure the edge zone is always on top of the software home button - detecting in the edge_swipe_detector when we are in landscape fullscreen mode + handling events from the right edge of the screen + dispatching back the events to the software home button instead of forwarding them to the app iframe in this case Let me know if it makes sense, cheers! ::: apps/system/js/software_button_manager.js @@ +252,5 @@ > + this.fullscreenButtonsElement.classList.add('visible'); > + window.setTimeout(() => { > + this.fullscreenButtonsElement.classList.remove('noTransition'); > + this.fullscreenButtonsElement.classList.remove('visible'); > + }, 500); we might want to figure out something more robust here, maybe based on waitForNextPaint @@ +353,5 @@ > + this._fullscreenClickShowsHome = true; > + }, 2500); > + this._fullscreenTimerId = window.setTimeout(() => { > + this.fullscreenButtonsElement.classList.remove('visible'); > + }, 2000); I have a feeling those timeouts might be tweaked later on, better make them constants right away :)
Attached file Github patch (deleted) —
Attachment #8448676 - Attachment is obsolete: true
(In reply to Rob MacDonald [:robmac] from comment #2) > Comment on attachment 8448676 [details] [diff] [review] > Change-software-home-button-location-in-landscape.patch > > Hi Markus... > > I downloaded the patch but I received a lot of errors. Talking to one of the > engineers he said the code may be out of date. Please NI me if there is an > updated patch UX should be reviewing. > > Also, fyi, within UX we're more used to downloading patches from github. Not > sure if this is an option for you but, if it is, it would definitely speed > up UX reviews. :) > > Thanks! > Rob Hi Rob, I've added a github pull request. BR, Markus
Flags: needinfo?(rmacdonald)
(In reply to Etienne Segonzac (:etienne) from comment #3) > Comment on attachment 8448676 [details] [diff] [review] > Change-software-home-button-location-in-landscape.patch > > Review of attachment 8448676 [details] [diff] [review]: > ----------------------------------------------------------------- > > Hey Markus, this is really promising! > I know there's still some ongoing UX work but maybe we can start tackling > the edge gesture issue in the meantime. > > There's already code in the edge swipe detector to differentiate between a > swipe and a tap/long tap so we can probably re-use that by > - making sure the edge zone is always on top of the software home button > - detecting in the edge_swipe_detector when we are in landscape fullscreen > mode + handling events from the right edge of the screen > + dispatching back the events to the software home button instead of > forwarding them to the app iframe in this case > > Let me know if it makes sense, cheers! > > ::: apps/system/js/software_button_manager.js > @@ +252,5 @@ > > + this.fullscreenButtonsElement.classList.add('visible'); > > + window.setTimeout(() => { > > + this.fullscreenButtonsElement.classList.remove('noTransition'); > > + this.fullscreenButtonsElement.classList.remove('visible'); > > + }, 500); > > we might want to figure out something more robust here, maybe based on > waitForNextPaint > > @@ +353,5 @@ > > + this._fullscreenClickShowsHome = true; > > + }, 2500); > > + this._fullscreenTimerId = window.setTimeout(() => { > > + this.fullscreenButtonsElement.classList.remove('visible'); > > + }, 2000); > > I have a feeling those timeouts might be tweaked later on, better make them > constants right away :) Hello Etienne. Please take a look at the pull request I uploaded to github, it contains a few more fixes than the patch I added before. One of the changes is that I split the right edge area into two and in landscape they doesn't cover the home button any more. That was just a simple solution to solve the problem, but maybe your suggestion to forward the taps is a better/nicer solution. I'll try to look into it. Regarding the 500ms delay after going to fullscreen, I know the comment preceding that code indicates that we want to act as soon as possible, but if it's hidden immediately you'll sometimes have a problem to catch everything that's going on (if the fullscreen view is alot different from the previous view). The slight delay lets the user notice what's going on. But that's just my 2 cents, maybe the ux people has another opinion :) BR, Markus
Flags: needinfo?(etienne)
(In reply to Markus Nilsson from comment #6) > Please take a look at the pull request I uploaded to github, it contains a > few more fixes than the patch I added before. One of the changes is that I > split the right edge area into two and in landscape they doesn't cover the > home button any more. That was just a simple solution to solve the problem, > but maybe your suggestion to forward the taps is a better/nicer solution. > I'll try to look into it. It works pretty well :) But a lot of swipes originate from the middle of the screen (on the y axis) so I think we want this part of the edge to work too. I think Rob also mentioned making the the edge zone wider in this case, so that if the user swipes from the edge of the software home bar instead of the edge of the screen it'll still work. > > Regarding the 500ms delay after going to fullscreen, I know the comment > preceding that code indicates that we want to act as soon as possible, but > if it's hidden immediately you'll sometimes have a problem to catch > everything that's going on (if the fullscreen view is alot different from > the previous view). The slight delay lets the user notice what's going on. > But that's just my 2 cents, maybe the ux people has another opinion :) We can maybe use a delay on the css transition then, but it's a detail. While I'm on the topic of CSS transition, you're currently transitioning the |bottom| or |right| property to show/hide the software home bar which is pretty bad for performance. You should transition on the |transform| property and do a |transform: translateY(100%)| or |transform: translateX(100%)| to get the bar out of the viewport depending on the orientation. If you go to the Settings app -> Developer, you'll find the very useful "Flash repainted area" option, currently when the bar gets in or out of the screen it's constantly flashing (being repainted, and causing reflows). With a transform it will stay the same color during the (much smoother) transition. Other useful perf tips: https://wiki.mozilla.org/FirefoxOS/Performance/App_Performance_Validation#2._Reflows.2FRestyles
Flags: needinfo?(etienne)
I tested a tap forwarding solution, but I don't think that it worked very well. The problem was the 300ms delay that meant that you couldn't tap the home button in a normal way, you had to hold the button a short while and when I did it was a little hard to judge for how long it should be held and the "open apps selector" was sometimes triggered instead. What I've done instead is to improve my previous solution so that the swipe area completely surrounds the home button and covers the entire button bar. So you can swipe from the end of the button bar as well as from the edge in the middle of the screen. Regarding the css transitions, the reason I did the bottom/right was because the app screen was also resized with an animation. But now I've changed it so only the button bar is animated and the performance is indeed much better. The github pull request is updated, please take another look.
Flags: needinfo?(etienne)
Blocks: 1037261
No longer blocks: soft-home-button
Hey Markus, can you split your patch with just the "soft home on the right" part on bug 1037261. This way we can focus on the event forwarding issue and deal with the fullscreen cases separately here. I'd like to get the first part (bug 1037261) landed this week/early next week :)
Flags: needinfo?(etienne)
NI'ing Francis on this patch as well. Thanks Francis!
Flags: needinfo?(fdjabri)
Attached video show_hide.mov (deleted) —
After applying this patch, unfortunately I was not able to notice any show/hide behavior in full screen mode.
Flags: needinfo?(fdjabri)
Hi Francis. In your movie it looks like you run the "Video" app and that it isn't really fullscreen. Try the "UI tests" app and do mozRequestFullScreen and/or Fullscreen Ctr.
Flags: needinfo?(fdjabri)
I pulled out the fullscreen soft home code and attached it to Bug 1037255, please review that instead.
Flags: needinfo?(rmacdonald)
Flags: needinfo?(fdjabri)
This bug report was split into Bug 1037261 and Bug 1037255. Closing with WONTFIX.
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: