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

pagecontainerbeforehide and pagecontainerbeforeshow data should contain both pages #7176

Closed
gabrielschulhof opened this Issue Feb 25, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@gabrielschulhof
Contributor

gabrielschulhof commented Feb 25, 2014

... otherwise, if you trap pagecontainerbeforeshow, you have no idea which page is being shown, because the event is triggered on the pagecontainer, not the page being shown.

Conversely, if you trap pagecontainerbeforehide, you have no idea which page is being hidden, because the event is triggered on the pagecontainer, not the page being hidden.

Currently (as of post-1.4.1), your only course of action is to listen to the deprecated pagebeforehide and pagebeforeshow events.

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Feb 25, 2014

Contributor

The function that needs changing:

var samePage = false;

Contributor

gabrielschulhof commented Feb 25, 2014

The function that needs changing:

var samePage = false;

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Mar 3, 2014

Contributor

This will also fix events pagecontainershow and pagecontainerhide which will improve the fix for #7085, because in the handler defined in the demo at demos/toolbar-fixed-persistent/ we no longer have to grab the active page via $( ".ui-page-active" ), but we can grab it via ui.nextPage, where ui is the second parameter to the event handler.

Contributor

gabrielschulhof commented Mar 3, 2014

This will also fix events pagecontainershow and pagecontainerhide which will improve the fix for #7085, because in the handler defined in the demo at demos/toolbar-fixed-persistent/ we no longer have to grab the active page via $( ".ui-page-active" ), but we can grab it via ui.nextPage, where ui is the second parameter to the event handler.

@jtara

This comment has been minimized.

Show comment
Hide comment
@jtara

jtara Mar 4, 2014

It seems to me this is just a nicety, since (presumably?) you can rely on fetching the active page in these two instances. (And that would be the workaround for now...)

It certainly would be nice, though, for the parameters to be consistent.

Aside: The wordiness of working with the new events is painful. Most of us will wind-up either keeping a more terse variable around in our own JS, or simply letting events bubble up to $(document) or using $("html").) I guess this deserves a separate Issue. It is certainly painful and non-DRY the way the currently examples (where they exist and are consistent...) are written. I'd think it hard to rationalize to developers why they now are being told to use a long string of nonsense when they've lived in the land of milk-and-honey xxxxxxxx $.mobile.activePage(), $.mobile.ChangePage() etc. What was once clear and straight-forward is now a jaw-numbing finger-punishing tongue-twister...

I understand the reasons for the internal structure. But does it have to be exposed in (only) such a raw way to developers? There has to be a better way.

(I'll do a separate Issue on this wordiness issue after some thought, and checking for existing Issues. This has been really bugging me... It's a serious blocker to clarity.)

jtara commented Mar 4, 2014

It seems to me this is just a nicety, since (presumably?) you can rely on fetching the active page in these two instances. (And that would be the workaround for now...)

It certainly would be nice, though, for the parameters to be consistent.

Aside: The wordiness of working with the new events is painful. Most of us will wind-up either keeping a more terse variable around in our own JS, or simply letting events bubble up to $(document) or using $("html").) I guess this deserves a separate Issue. It is certainly painful and non-DRY the way the currently examples (where they exist and are consistent...) are written. I'd think it hard to rationalize to developers why they now are being told to use a long string of nonsense when they've lived in the land of milk-and-honey xxxxxxxx $.mobile.activePage(), $.mobile.ChangePage() etc. What was once clear and straight-forward is now a jaw-numbing finger-punishing tongue-twister...

I understand the reasons for the internal structure. But does it have to be exposed in (only) such a raw way to developers? There has to be a better way.

(I'll do a separate Issue on this wordiness issue after some thought, and checking for existing Issues. This has been really bugging me... It's a serious blocker to clarity.)

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Mar 6, 2014

Contributor

@jtara, the widget factory allows you to attach event handlers via the constructor mechanism:

$( ":data('mobile-pagecontainer')" ).pagecontainer({
  beforeshow: function( event, ui ) {},
  beforehide: function( event, ui ) {},
  show: function( event, ui ) {},
  hide: function( event, ui ) {}
});

This way you can avoid some of the wordiness.

Contributor

gabrielschulhof commented Mar 6, 2014

@jtara, the widget factory allows you to attach event handlers via the constructor mechanism:

$( ":data('mobile-pagecontainer')" ).pagecontainer({
  beforeshow: function( event, ui ) {},
  beforehide: function( event, ui ) {},
  show: function( event, ui ) {},
  hide: function( event, ui ) {}
});

This way you can avoid some of the wordiness.

@SteveVaneeckhout

This comment has been minimized.

Show comment
Hide comment
@SteveVaneeckhout

SteveVaneeckhout Apr 13, 2014

It seems to me this is just a nicety, since (presumably?) you can rely on fetching the active page in these two instances. (And that would be the workaround for now...)

That is true for pagecontainerbeforeshow but not pagecontainerbeforehide.

SteveVaneeckhout commented Apr 13, 2014

It seems to me this is just a nicety, since (presumably?) you can rely on fetching the active page in these two instances. (And that would be the workaround for now...)

That is true for pagecontainerbeforeshow but not pagecontainerbeforehide.

arschmitz added a commit that referenced this issue Jun 16, 2014

Pagecontainer: make all page events go through triggerWithDeprecated
To make sure all former page events have a page container counterpart,
and that this counterpart has both a toPage and prevPage prop on the ui
object
Also add a demo of how to debug pagecontainer events

Fixes gh-7063
Fixes gh-70283
Fixes gh-7176
Closes gh-7285

arschmitz added a commit that referenced this issue Jun 17, 2014

Pagecontainer: make all page events go through triggerWithDeprecated
To make sure all former page events have a page container counterpart,
and that this counterpart has both a toPage and prevPage prop on the ui
object
Also add a demo of how to debug pagecontainer events

Fixes gh-7063
Fixes gh-7283
Fixes gh-7176
Closes gh-7285

@arschmitz arschmitz closed this in 3d4d7dd Jun 18, 2014

arschmitz added a commit that referenced this issue Jun 18, 2014

Pagecontainer: make all page events go through triggerWithDeprecated
To make sure all former page events have a page container counterpart,
and that this counterpart has both a toPage and prevPage prop on the ui
object
Also add a demo of how to debug pagecontainer events

Fixes gh-7063
Fixes gh-7283
Fixes gh-7176
Closes gh-7285

(cherry picked from commit 3d4d7dd)

agcolom added a commit to agcolom/jquery-mobile that referenced this issue Nov 26, 2014

Pagecontainer: make all page events go through triggerWithDeprecated
To make sure all former page events have a page container counterpart,
and that this counterpart has both a toPage and prevPage prop on the ui
object
Also add a demo of how to debug pagecontainer events

Fixes gh-7063
Fixes gh-7283
Fixes gh-7176
Closes gh-7285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment