Fix link togglers jumping to top
What does this MR do?
-
e.preventDefault()only foraandbutton. We want the checkbox to do its own thing.
Using <button> is semantically more accurate for the usage and we don't need the previous e.preventDefault()
Are there points in the code the reviewer needs to double check?
Togglers to check:
- "Create a Mattermost team for this group" checkbox, http://localhost:3000/groups/new (enable
mattermostinconfig/gitlab.yml) - "Repo by Url" button, http://localhost:3000/projects/new
- In MR widget "Modify commit message", http://localhost:3000/gitlab-org/gitlab-ce/merge_requests/3
- Discussion "Toggle discussion", http://localhost:3000/gitlab-org/gitlab-ce/merge_requests/3
- "Showing 1 changed file with 4 additions and 7 deletions" , http://localhost:3000/gitlab-org/gitlab-ce/merge_requests/3/diffs
- Commit description toggle "...", http://localhost:3000/gitlab-org/gitlab-ce/commits/master
Why was this MR needed?
- We used
atags with empty fragmenthref="#"which would jump you to the top of the page when clicked
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added - Tests
-
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #29414 (closed)
Merge request reports
Activity
mentioned in merge request !9918 (merged)
21 21 // %a.js-toggle-button 22 22 // %div.js-toggle-content 23 23 // 24 $('body').on('click', '.js-toggle-button', function() { 24 $('body').on('click', '.js-toggle-button', function(e) { 25 25 toggleContainer($(this).closest('.js-toggle-container')); 26 27 const targetTag = e.target.tagName.toLowerCase(); 28 if (targetTag === 'a' || targetTag === 'button') { - Prevent links from changing the URL (not really necessary in the future because we use buttons everywhere in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9918)
- Prevent buttons from submitting forms
Edited by username-removed-892863
Why do we need to check the tag name? Can't we just do
e.preventDefault()? Is there a situation where we want this @MadLittleMods?@jschatz1 Yes, we want checkboxes to check, https://gitlab.slack.com/archives/frontend/p1488925034023752
This is a great example why we should avid doing
<a href="#">for clickable elements other than linksEdited by username-removed-408881
enabled an automatic merge when the pipeline for f97c1d10 succeeds
mentioned in commit 8fddde5b
mentioned in issue #29538 (closed)
mentioned in issue #29527 (closed)
mentioned in commit d4c8e847
mentioned in merge request !10126 (merged)
