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.

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

Cancel button on modal demo #331

Closed
wants to merge 1 commit into from
Closed

Conversation

pamelafox
Copy link
Contributor

I realized you can easily make a cancel button by adding close class to the secondary button. From his tweet to me, I think @mdo is a fan of providing a cancel button in the dialog. If that's a best practice, maybe it should be in the docs? (Or if you think cancel buttons are overkill, then that's cool. Your call).

…to the secondary button. From his tweet to me, I think @mdo is a fan of providing a cancel button in the dialog. If that's a best practice, maybe it should be in the docs? (Or if you think cancel buttons are overkill, then that's cool. Your call).
@pamelafox
Copy link
Contributor Author

Ah it's not so easy, as then the button gets all the 'close' CSS. I've added a 'close-button' class to my HTML and the modal JS for now. It could also look for button type="reset" as indication of a cancel button..but that might be presuming too much.

@mdo
Copy link
Member

mdo commented Sep 29, 2011

A good practice I've started to adopt elsewhere is .js-* for the behavior class. This helps remove the behavior changes of javascript from the display of CSS.

Thoughts?

@pamelafox
Copy link
Contributor Author

Ah, so what you're saying is that there'd be a "js-close" class and that'd apply both to the cancel button and to the 'X'? (in addition to other classes on them). That makes sense, though you'd probably want to check that system makes sense for all the JS libs.

@marcalj
Copy link

marcalj commented Sep 30, 2011

+1 to use some prefix for JS related functionalities.

@fat
Copy link
Member

fat commented Oct 3, 2011

Hm... think we're going to stick to a data-attribute driven pubsub model. Going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants