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

Remove support for input elements #11

Closed
wants to merge 1 commit into from
Closed

Remove support for input elements #11

wants to merge 1 commit into from

Conversation

AntonTrollback
Copy link
Contributor

Fixes #6

@simonsmith
Copy link
Member

I know it's ancient but I'd vote to merge this, @timkelty @giuseppeg?

elements. For `a` elements, you should remove the `href` attribute and prevent
JavaScript event handlers from firing.
N.B. You must also include the `disabled` attribute on `button` elements. For
`a` elements, you should remove the `href` attribute and prevent JavaScript
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use a span then? I try to advocate this as much as possible but it is a tough one to win :)

@giuseppeg
Copy link
Member

@simonsmith sounds good to me

@timkelty
Copy link
Member

timkelty commented Oct 1, 2015

I don't think we should be endorsing an a with no href (or a clickable span).

If it's clickable, it should be a <button> element. If its in a form and you don't want it to submit, you do <button type="button">.

My vote is to just wipe that whole line about a elements.

As for dropping input support, I'm on board.

@AntonTrollback
Copy link
Contributor Author

+1 merge and drop the "a" line

@simonsmith
Copy link
Member

Preventing default events is a valid use of JS I would say, but removing the href not so much.

@simonsmith
Copy link
Member

https://github.com/suitcss/components-button/compare/PR/11 Look good? I'm thinking a major version bump too

@timkelty
Copy link
Member

timkelty commented Oct 2, 2015

:shipit:

@simonsmith
Copy link
Member

Merged in 7d8936c

@simonsmith simonsmith closed this Oct 2, 2015
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.

Drop support for input
4 participants