Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix ESC button bug #6

Merged
merged 4 commits into from Oct 26, 2011
Merged

Fix ESC button bug #6

merged 4 commits into from Oct 26, 2011

Conversation

elidupuis
Copy link
Contributor

...see below.

@elidupuis
Copy link
Contributor Author

A few small tweaks:

  • 7c415ad merge two addClass calls into one.
  • 59c58be each node in the ajax content was getting it's own modal. just wrapping the ajax return in a div here.
  • 77fbf5f current class was not getting removed from modal content when modal was closed.

kylefox added a commit that referenced this pull request Oct 26, 2011
@kylefox
Merge pull request #6 from elidupuis/master
0e1cadc
@kylefox kylefox merged commit 0e1cadc into kylefox:master Oct 26, 2011
@kylefox
Copy link
Owner

kylefox commented Oct 26, 2011

Nice work, thanks for the patches! Good catch on 59c58be, not sure how the hell I missed that...

Re: your event name, keydown.modal, how do you feel about sticking to the existing convention, ex: modal:keydown ?

@elidupuis
Copy link
Contributor Author

I'm fine with the custom convention, but I'm not exactly sure how you'd pull it off... We still need to bind to the standard keydown event, do we not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@elidupuis @kylefox