Skip to content

Use <button> instead of <a> for JavaScript interactive buttons #9892

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

Merged
merged 1 commit into from
Jun 11, 2018

Conversation

Calinou
Copy link
Contributor

@Calinou Calinou commented Jun 6, 2018

This removes the need to call preventDefault() to disable the default action of <a> elements, since <button type="button"> has no default action whatsoever.

See also this Stack Overflow question about the <button> HTML tag.

This removes the need to call `preventDefault()` to disable the default
action of <a> elements, since <button type="button"> has no default
action whatsoever.
@javiereguiluz
Copy link
Member

@Calinou this looks really nice to me! Thank you. I'll give some time before merging so others can review and comment on this.

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Thanks for this PR!

@HeahDude HeahDude added this to the 2.8 milestone Jun 10, 2018
@javiereguiluz
Copy link
Member

Thanks Hugo.

@javiereguiluz javiereguiluz merged commit 87db1b6 into symfony:2.8 Jun 11, 2018
javiereguiluz added a commit that referenced this pull request Jun 11, 2018
…ttons (Calinou)

This PR was merged into the 2.8 branch.

Discussion
----------

Use <button> instead of <a> for JavaScript interactive buttons

This removes the need to call `preventDefault()` to disable the default action of `<a>` elements, since `<button type="button">` has no default action whatsoever.

See also [this Stack Overflow question](https://stackoverflow.com/questions/469059/button-vs-input-type-button-which-to-use) about the `<button>` HTML tag.

Commits
-------

87db1b6 Use <button> instead of <a> for JavaScript interactive buttons
@@ -353,8 +353,8 @@ will be show next):
var $collectionHolder;

// setup an "add a tag" link
var $addTagLink = $('<a href="#" class="add_tag_link">Add a tag</a>');
var $newLinkLi = $('<li></li>').append($addTagLink);
var $addTagButton = $('<button type="button" class="add_tag_link">Add a tag</a>');
Copy link
Contributor

Choose a reason for hiding this comment

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

The closing tag should be updated as well: <button>...</a>

$removeFormA.on('click', function(e) {
// prevent the link from creating a "#" on the URL
e.preventDefault();
var $removeFormButton = $('<button type="button">Delete this tag</a>');
Copy link
Contributor

Choose a reason for hiding this comment

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

Here as well

@apfelbox
Copy link
Contributor

Both inline comments adressed in #9911

javiereguiluz added a commit that referenced this pull request Jun 11, 2018
This PR was merged into the 2.8 branch.

Discussion
----------

Use matching closing tag

Fixes a small typo which was missed in #9892

Commits
-------

f031cf5 Use matching closing tag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants