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

Closing Dialog opened from Popup redirects to first screen in screen chain #7538

Closed
dgruzda opened this Issue Jul 2, 2014 · 3 comments

Comments

Projects
None yet
2 participants
@dgruzda

dgruzda commented Jul 2, 2014

1 Add Foo, Bar pages and Baz dialog
2 Add link for opening popup from Bar screen
3 Add link for opening Baz dialog from popup

Expected Result: 
After Baz Dialog has closed by Close button we stay on screen Bar
Actual Result: After Baz Dialog has closed by Close button we are on screen Foo
http://jsbin.com/jivaluli/2

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Jul 2, 2014

Contributor

This worked fine in 1.3.2: http://jsbin.com/jivaluli/4

Contributor

gabrielschulhof commented Jul 2, 2014

This worked fine in 1.3.2: http://jsbin.com/jivaluli/4

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Jul 2, 2014

Contributor

The defining characteristic of a dialog widget (not a page styled as a dialog) is that nav opens it using special logic, rendering the URL unreachable via dialogHashKey. On the flipside, when a navigate event occurs onto a history entry that has the dialogHashKey, nav invokes the dialog special logic. That logic checks whether the current page is a dialog, and if so, it navigates to the previous dialog. Otherwise, it skips over the current history entry via back() or forward().

The problem is that it checks whether the current page is a dialog by checking whether the current page has the class ui-dialog. This is incorrect, because a page widget with the dialog option set to true also has this class.

In the above sequence, the popup causes a history entry with a dialogHashKey to appear, followed by a history entry containing #baz, which is a page styled as a dialog, not a dialog widget. When you navigate back() from the dialog, the current page contains the class ui-dialog, and the hash contains the dialogHashKey, making it appear that you are on a dialog page. However, this is not the case, and you must not handle the navigate event using the dialog special logic.

Thus, we need to modify the check that identifies the current page as a dialog. Since the presence/absence of the ui-dialog class is no longer indicative, let's use .is( ":data(mobile-dialog)" ) instead. This needs to be marked as deprecated as of 1.4.0 and it needs to be removed for 1.5.0.

Contributor

gabrielschulhof commented Jul 2, 2014

The defining characteristic of a dialog widget (not a page styled as a dialog) is that nav opens it using special logic, rendering the URL unreachable via dialogHashKey. On the flipside, when a navigate event occurs onto a history entry that has the dialogHashKey, nav invokes the dialog special logic. That logic checks whether the current page is a dialog, and if so, it navigates to the previous dialog. Otherwise, it skips over the current history entry via back() or forward().

The problem is that it checks whether the current page is a dialog by checking whether the current page has the class ui-dialog. This is incorrect, because a page widget with the dialog option set to true also has this class.

In the above sequence, the popup causes a history entry with a dialogHashKey to appear, followed by a history entry containing #baz, which is a page styled as a dialog, not a dialog widget. When you navigate back() from the dialog, the current page contains the class ui-dialog, and the hash contains the dialogHashKey, making it appear that you are on a dialog page. However, this is not the case, and you must not handle the navigate event using the dialog special logic.

Thus, we need to modify the check that identifies the current page as a dialog. Since the presence/absence of the ui-dialog class is no longer indicative, let's use .is( ":data(mobile-dialog)" ) instead. This needs to be marked as deprecated as of 1.4.0 and it needs to be removed for 1.5.0.

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Jul 2, 2014

Contributor

OK, .data( "mobile-dialog" ) is probably faster.

Contributor

gabrielschulhof commented Jul 2, 2014

OK, .data( "mobile-dialog" ) is probably faster.

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