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

Fix from #5382 causes failure: Full page dialog does not receive rounded corners - remove the fix and the rounded corners come back #5383

Closed
ShamimIslam opened this Issue Dec 17, 2012 · 4 comments

Comments

Projects
None yet
2 participants
@ShamimIslam

ShamimIslam commented Dec 17, 2012

  1. Issue description
    Dialog pages (data-rel=dialog) pages with a popup (data-role=popup) fail to respond to the close button of the Dialog regardless of the state of the popup.
  2. Test page
    http://jsfiddle.net/ShamimIslam/Y7VJC/
  3. Steps to reproduce
    a. Create a JQM page, A with header, footer and a link, B (data-rel=dialog) to a second JQM page, C
    b. Create the second JQM page, C with header, a footer and a link, D, to a popup div E (data-role=popup)
    c. Create the popup div E in the content section of C.
    d. Put a header and content into E
    e. When done, the nesting should look like JQMDoc(A(header,content(B),footer),C(header,content(D,E),footer))
    f. Open the page and click the link B to get to the dialog C
  4. Expected outcome
    The full page dialog has square corners - similer to the demos on Jquerymobile.com in the dialog section
  5. Actual outcome
    The full page dialog will have rounded corners - unlike the demos on Jquerymobile. com in the dialog section
  6. Firefox 17.0.1/Fedora 17/x86_64, Android Browser/Android ICS 4.0.4/Razr Maxx
  7. JQM 1.2.0, JQuery 1.8.2
  8. The previous fix to repair the close button behavior created this issue. Can also be seen when the structure is JQMDoc(A(header,content(B),footer),C(header,content,footer))
@ShamimIslam

This comment has been minimized.

Show comment
Hide comment
@ShamimIslam

ShamimIslam Dec 17, 2012

Nm - seems to work on the CDN version - may be something in the fixed .js

ShamimIslam commented Dec 17, 2012

Nm - seems to work on the CDN version - may be something in the fixed .js

@ShamimIslam

This comment has been minimized.

Show comment
Hide comment
@ShamimIslam

ShamimIslam Dec 17, 2012

Fix from issue #5382 removes the rounded corners from the dialog C, but the popup E retains the rounded dialogs.

There seems to be a cascaded effect here.

ShamimIslam commented Dec 17, 2012

Fix from issue #5382 removes the rounded corners from the dialog C, but the popup E retains the rounded dialogs.

There seems to be a cascaded effect here.

@ShamimIslam ShamimIslam reopened this Dec 17, 2012

@ShamimIslam

This comment has been minimized.

Show comment
Hide comment
@ShamimIslam

ShamimIslam Dec 17, 2012

Found the fix, don't know how to commit it.

Before fix:
.prepend().end()

After fix:
.first().prepend().end()

Correct fix:
.first().prepend().end().end()

.first() is a filter and as such needs to be removed before the rest of the chained actions.

ShamimIslam commented Dec 17, 2012

Found the fix, don't know how to commit it.

Before fix:
.prepend().end()

After fix:
.first().prepend().end()

Correct fix:
.first().prepend().end().end()

.first() is a filter and as such needs to be removed before the rest of the chained actions.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Dec 17, 2012

Member

@ShamimIslam

I fixed the original issue on branch master (pre 1.3) and applied the same fix on branch 1.2-stable for the 1.2.1 release. Because we changed the way we apply corner styling for 1.3 the code on 1.2-stable is a bit different and I missed that I had to add another .end() there. Thanks a lot!

Closing as fixed by 113e3b4

Member

jaspermdegroot commented Dec 17, 2012

@ShamimIslam

I fixed the original issue on branch master (pre 1.3) and applied the same fix on branch 1.2-stable for the 1.2.1 release. Because we changed the way we apply corner styling for 1.3 the code on 1.2-stable is a bit different and I missed that I had to add another .end() there. Thanks a lot!

Closing as fixed by 113e3b4

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