New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

about lastscroll in the nightly version #1774

Closed
badtoto opened this Issue Jun 3, 2011 · 13 comments

Comments

Projects
None yet
7 participants
@badtoto

badtoto commented Jun 3, 2011

http://jsbin.com/oriva3/2

lastscroll doesn't work

@globalmatt

This comment has been minimized.

Show comment
Hide comment
@globalmatt

globalmatt Jun 30, 2011

Contributor

Agreed. The scrollback function seems to be broken in beta 1 / latest:

http://jsbin.com/ewiwet/

Scroll down, tap Next Page, then tap the browser Back button to return to the first page.

Expected behaviour: JQM scrolls the page down to the Next Page link.
Actual behaviour: The user is returned to the top of the first page.

This is on iPhone 4 / iOS 4.3.3.

The scrollback works on 1.0a4.1.

Contributor

globalmatt commented Jun 30, 2011

Agreed. The scrollback function seems to be broken in beta 1 / latest:

http://jsbin.com/ewiwet/

Scroll down, tap Next Page, then tap the browser Back button to return to the first page.

Expected behaviour: JQM scrolls the page down to the Next Page link.
Actual behaviour: The user is returned to the top of the first page.

This is on iPhone 4 / iOS 4.3.3.

The scrollback works on 1.0a4.1.

@jasonaward

This comment has been minimized.

Show comment
Hide comment
@jasonaward

jasonaward Jul 19, 2011

From what it appears... if you create a button instead of a standard link, the last scroll functionality works fine (in my testing environment of Firefox), but if you create a standard link, last scroll does not work properly.

I've been trying to dig through the code to debug this further, but at first glance it's a bit overwhelming.

jasonaward commented Jul 19, 2011

From what it appears... if you create a button instead of a standard link, the last scroll functionality works fine (in my testing environment of Firefox), but if you create a standard link, last scroll does not work properly.

I've been trying to dig through the code to debug this further, but at first glance it's a bit overwhelming.

@jasonaward

This comment has been minimized.

Show comment
Hide comment
@jasonaward

jasonaward Jul 20, 2011

Well, it appears that a button works because it has the .ui-btn class attached to it. A silly test of adding the ui-btn class to any standard link makes that link work. As for an immediate fix, I simply changed this line:

            $activeClickedLink = $link.closest( ".ui-btn" );

... to ...

            $activeClickedLink = $link.closest( "a" );

I'm not sure if that's what was intended, but for me, it appears to be doing the job.

jasonaward commented Jul 20, 2011

Well, it appears that a button works because it has the .ui-btn class attached to it. A silly test of adding the ui-btn class to any standard link makes that link work. As for an immediate fix, I simply changed this line:

            $activeClickedLink = $link.closest( ".ui-btn" );

... to ...

            $activeClickedLink = $link.closest( "a" );

I'm not sure if that's what was intended, but for me, it appears to be doing the job.

@jasonaward

This comment has been minimized.

Show comment
Hide comment
@jasonaward

jasonaward Jul 20, 2011

Hopefully one final update to this... a better selector would probably be:

            $activeClickedLink = $link.closest( ".ui-btn, .ui-link" );

That will probably be closer to what was intended.

jasonaward commented Jul 20, 2011

Hopefully one final update to this... a better selector would probably be:

            $activeClickedLink = $link.closest( ".ui-btn, .ui-link" );

That will probably be closer to what was intended.

@globalmatt

This comment has been minimized.

Show comment
Hide comment
@globalmatt

globalmatt Jul 31, 2011

Contributor

@jasonaward : That looks like a good fix to me!

Contributor

globalmatt commented Jul 31, 2011

@jasonaward : That looks like a good fix to me!

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Jul 31, 2011

Contributor

Thanks! I'll take a look and try and get this (or another) fix in for b2.

Contributor

scottjehl commented Jul 31, 2011

Thanks! I'll take a look and try and get this (or another) fix in for b2.

@ghost ghost assigned scottjehl Jul 31, 2011

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Aug 7, 2011

Contributor

So, lastScroll no longer works primarily because we stored the lastScroll data on the page element, which is now removed after pagehide. Also, storing a reference to $activeClickedLink as shown above will simply leak memory on most pages (aside from returning to home), since a page and its child elements are removed from the DOM on pagehide.

I've started a branch (https://github.com/jquery/jquery-mobile/tree/last-scroll) with one commit that does the following.

  • This change sets the "lastScroll" property to each urlHistory item object, allowing us to remember previous scroll distances when returning to a page that has since been removed from the DOM. Before this change, this number was stored in data on the page element, so it is lost when the page is removed after pagehide.
  • Also, this change removes a reference in memory that we were keeping to the $activeClickedLink on each page. We stored this in attempt to refocus a link after returning to a page. Unfortunately, it doesn't seem that this data can be retained after pages are removed from the DOM, outside of somehow remembering a unique selector string to reach that element again (which could be achieved by adding some overhead, ala http://stackoverflow.com/questions/2068272/getting-a-jquery-selector-for-an-element )

The diff is here: https://github.com/jquery/jquery-mobile/compare/last-scroll

Feedback is much appreciated. thanks!

Contributor

scottjehl commented Aug 7, 2011

So, lastScroll no longer works primarily because we stored the lastScroll data on the page element, which is now removed after pagehide. Also, storing a reference to $activeClickedLink as shown above will simply leak memory on most pages (aside from returning to home), since a page and its child elements are removed from the DOM on pagehide.

I've started a branch (https://github.com/jquery/jquery-mobile/tree/last-scroll) with one commit that does the following.

  • This change sets the "lastScroll" property to each urlHistory item object, allowing us to remember previous scroll distances when returning to a page that has since been removed from the DOM. Before this change, this number was stored in data on the page element, so it is lost when the page is removed after pagehide.
  • Also, this change removes a reference in memory that we were keeping to the $activeClickedLink on each page. We stored this in attempt to refocus a link after returning to a page. Unfortunately, it doesn't seem that this data can be retained after pages are removed from the DOM, outside of somehow remembering a unique selector string to reach that element again (which could be achieved by adding some overhead, ala http://stackoverflow.com/questions/2068272/getting-a-jquery-selector-for-an-element )

The diff is here: https://github.com/jquery/jquery-mobile/compare/last-scroll

Feedback is much appreciated. thanks!

@ghost ghost assigned jblas and scottjehl Aug 7, 2011

@toddparker

This comment has been minimized.

Show comment
Hide comment
@toddparker

toddparker Aug 7, 2011

Contributor

Scott - should we merge in this branch? Been a while and no word from anyone on this ticket.

Contributor

toddparker commented Aug 7, 2011

Scott - should we merge in this branch? Been a while and no word from anyone on this ticket.

@jasonaward

This comment has been minimized.

Show comment
Hide comment
@jasonaward

jasonaward Aug 29, 2011

This appears to be working correctly (including the latest changes to the branch from 2 days ago) from our testing.

Let me know if you have any questions before this can be pushed into master. Thanks!

jasonaward commented Aug 29, 2011

This appears to be working correctly (including the latest changes to the branch from 2 days ago) from our testing.

Let me know if you have any questions before this can be pushed into master. Thanks!

@toddparker

This comment has been minimized.

Show comment
Hide comment
@toddparker

toddparker Sep 8, 2011

Contributor

Now fixed:
d68d506

Contributor

toddparker commented Sep 8, 2011

Now fixed:
d68d506

@toddparker toddparker closed this Sep 8, 2011

@nareneko

This comment has been minimized.

Show comment
Hide comment
@nareneko

nareneko Oct 3, 2011

I feel like this is still an issue in IE 7/8. I'm wondering if anyone else has noticed this?

nareneko commented Oct 3, 2011

I feel like this is still an issue in IE 7/8. I'm wondering if anyone else has noticed this?

@toddparker

This comment has been minimized.

Show comment
Hide comment
@toddparker

toddparker Oct 3, 2011

Contributor

@nareneko - Can you create a jsbin or jsfiddle we can test?

Contributor

toddparker commented Oct 3, 2011

@nareneko - Can you create a jsbin or jsfiddle we can test?

@nareneko

This comment has been minimized.

Show comment
Hide comment
@nareneko

nareneko Oct 3, 2011

I created a page with a listview of 30 items. If you scroll down to the bottom and click on one of the later items, it will display a dialog. When I click out of the dialog, it should take me back to the last scroll position of the first page of listview items. In IE 7 & 8, it doesn't seem to do this.

http://jsfiddle.net/R9Ze2/

nareneko commented Oct 3, 2011

I created a page with a listview of 30 items. If you scroll down to the bottom and click on one of the later items, it will display a dialog. When I click out of the dialog, it should take me back to the last scroll position of the first page of listview items. In IE 7 & 8, it doesn't seem to do this.

http://jsfiddle.net/R9Ze2/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment