Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#17017 closed defect (fixed)

[dojox/mobile] SwapView oddities

Reported by: Paul Christopher Owned by: Eric Durocher
Priority: undecided Milestone: 1.9
Component: DojoX Mobile Version: 1.9.0b2
Keywords: Cc:
Blocked By: Blocking:

Description

Strange offset top during swapping

Run the attached testcase. Swipe through the views and notice, that all SwapViews have a strange top padding / offset.

SwapView.show() does not remove padding-top / does not update PageIndicator

Now move to the last pane ('Pane 3'). Click on the button "Go to Pane1". Pane1 has always a large top padding. You need to call Pane1._removeTransitionPaddingTop() to get rid off that. Also notice: The PageIndicator is not updated. You need to call PageIndicator.reset().

SwapView.goTo() causes vertical scrollbar

Open the debug console and make it as large as possible so that the actual display port for the html page is only 10cm. Reload the page. Swipe through the pages until Pane3. Click "Slide to Pane1". The display port displays now a vertical scroll bar which causes the heading/ PageIndicator to move to the left by some pixel. As soon as you start moving through the panes again, the scrollbar disappears

Affected browser

Tests above conducted with Safari 5.1.7 / Dojo trunk only.

Related ticket

http://bugs.dojotoolkit.org/ticket/17001

Attachments (3)

test_SwapView.html (1.9 KB) - added by Paul Christopher 7 years ago.
ticket17017.patch (4.1 KB) - added by Sebastien Brunot 7 years ago.
SwapView is now emitting a /dojox/mobile/viewChanged event when its show method is called, and it is also correctly removing temporary padding added before transitions (IBM CCLA).
ticket17017-2.patch (1.1 KB) - added by Sebastien Brunot 7 years ago.
Revert the emission of a /dojox/mobile/viewChanged event when SwapView? show method is called, as it cause a regression in Carousel (IBM CCLA).

Download all attachments as: .zip

Change History (15)

Changed 7 years ago by Paul Christopher

Attachment: test_SwapView.html added

comment:1 Changed 7 years ago by Sebastien Brunot

I've been able to reproduce the problem with chrome on Windows 7, both with 1.9 and 1.8.

comment:2 Changed 7 years ago by Sebastien Brunot

the "Strange offset top during swapping" problem can be dealed with using the following published workaround: http://livedocs.dojotoolkit.org/dojox/mobile/faq#how-do-i-avoid-the-margin-collapsing-problem

Unfortunately, that doesn't solve the other problems...

Changed 7 years ago by Sebastien Brunot

Attachment: ticket17017.patch added

SwapView is now emitting a /dojox/mobile/viewChanged event when its show method is called, and it is also correctly removing temporary padding added before transitions (IBM CCLA).

comment:3 Changed 7 years ago by Sebastien Brunot

Patch attached to fix the problems on trunk.

comment:4 Changed 7 years ago by Eric Durocher

Resolution: fixed
Status: newclosed

In [31245]:

fixes #17017. SwapView? is now emitting a /dojox/mobile/viewChanged event when its show method is called, and it is also correctly removing temporary padding added before transitions. Thanks Sebastien Brunot (IBM, CCLA) !strict

comment:5 Changed 7 years ago by Eric Durocher

Milestone: tbd1.9

comment:6 Changed 7 years ago by Paul Christopher

Wow, that was quick! Thanks a lot for fixing this! Could you also reproduce the problem with the vertical scrollbar for very small viewport sizes, see last point "SwapView.goTo() causes vertical scrollbar"?

comment:7 Changed 7 years ago by Sebastien Brunot

Hi Paul,

unfortunately I cannot reproduce the issue because I don't currently have access to Safari 5.1.7. Could you try to set height="auto" on each of your views and see if that solves the problem ? Here's an example for view 1:

<div data-dojo-type="dojox/mobile/SwapView" data-dojo-id="Pane1" id="Pane1" height="auto">

Unfortunately, with the workaround to solve the strange offset top during swapping, the resize method of scollable doesn't take into account the border for calculating the view height and a vertical scroll bar appears on each view whatever the size of the viewport...

comment:8 Changed 7 years ago by Paul Christopher

dojox/mobile/themes/iphone/common.css defines the body like this:

.mobile body {
  overflow-x: hidden;
  -webkit-text-size-adjust: none;
  font-family: Helvetica;
  font-size: 17px;
}

To get rid off the vertical scrollbar, I had to set "overflow-y:hidden" as well. Is "overflow-y" left out on purpose in common.css or is this a bug?

P.S.: The vertical scrollbar does also show up in Firefox. It's not only a Webkit issue...

comment:9 Changed 7 years ago by Eric Durocher

To be honest I don't know for sure the reason. My guess is that {overflow-y: hidden} is not set just to leave the possibility for an application to create a page higher than the viewport and let the browser scroll it "natively", as an alternative to using a ScrollableView.

That said, I am not sure setting {overflow-y: hidden} ever actually prevents native vertical scrolling on mobile, although it should logically do it to be consistent with desktop... It does not on iOS, I did not test other devices.

Anyway, it is not something we can change quickly without careful testing, and there is a very easy workaround (i.e. add it in your app).

Note also that we plan to investigate re-implementing or replacing ScrollableView based on how modern mobile browsers support position:fixed, overflow on divs, etc, which could make ScrollableView's JavaScript scrolling unnecessary. So definitively something we will reconsider at least for 2.0.

comment:10 Changed 7 years ago by Sebastien Brunot

I've attached a second patch, ticket17017-2.patch, that fix a regression introduced in Carousel.js by the first patch.

Changed 7 years ago by Sebastien Brunot

Attachment: ticket17017-2.patch added

Revert the emission of a /dojox/mobile/viewChanged event when SwapView? show method is called, as it cause a regression in Carousel (IBM CCLA).

comment:11 Changed 7 years ago by Sebastien Brunot

The emission of the /dojox/mobile/viewChanged event in the show method is causing a regression in Carousel. As a consequence, I've published a second patch that revert it. The workaround is to call the reset() method on the PageIndicator after calling the show method on the SwapView.

comment:12 Changed 7 years ago by Eric Durocher

In [31333]:

refs #17017. Fix regression in Carousel caused by previous patch: backtrack firing viewChanged event in show method. Thanks Sebastien Brunot (IBM, CCLA)

Note: See TracTickets for help on using tickets.