diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-03-14 00:02:30 -0500 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-03-28 11:09:12 -0500 |
commit | cf3c43980d3911e32d767158420400d75edf3968 (patch) | |
tree | 18741697d64bf26a98e8e15b005062ecb4b454ed | |
parent | 6460489babe3cd25358800804f6befc1bd242f22 (diff) | |
download | gitlab-ce-cf3c43980d3911e32d767158420400d75edf3968.tar.gz |
Fix link togglers jumping to top
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/29414
-rw-r--r-- | app/assets/javascripts/behaviors/toggler_behavior.js | 2 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/diff.scss | 15 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/merge_requests.scss | 10 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/notes.scss | 14 | ||||
-rw-r--r-- | app/views/discussions/_discussion.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/commits/_commit.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/diffs/_stats.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/merge_requests/widget/open/_accept.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/new.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/29414-fix-toggle-discussion-link-jump.yml | 4 | ||||
-rw-r--r-- | features/steps/dashboard/new_project.rb | 2 | ||||
-rw-r--r-- | features/steps/project/merge_requests.rb | 2 | ||||
-rw-r--r-- | spec/features/merge_requests/diff_notes_resolve_spec.rb | 6 | ||||
-rw-r--r-- | spec/features/merge_requests/merge_commit_message_toggle_spec.rb | 2 |
14 files changed, 55 insertions, 12 deletions
diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index 86927314dd4..576b8a0425f 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -18,7 +18,7 @@ // Button does not change visibility. If button has icon - it changes chevron style. // // %div.js-toggle-container - // %a.js-toggle-button + // %button.js-toggle-button // %div.js-toggle-content // $('body').on('click', '.js-toggle-button', function(e) { diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index eab79c2a481..1aa1079903c 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -431,6 +431,21 @@ border-bottom: none; } +.diff-stats-summary-toggler { + padding: 0; + background-color: transparent; + border: 0; + color: $gl-link-color; + transition: color 0.1s linear; + + &:hover, + &:focus { + outline: none; + text-decoration: underline; + color: $gl-link-hover-color; + } +} + // Mobile @media (max-width: 480px) { .diff-title { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 6630904ec92..566dcc64802 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -60,7 +60,17 @@ } .modify-merge-commit-link { + padding: 0; + + background-color: transparent; + border: 0; + color: $gl-text-color; + + &:hover, + &:focus { + text-decoration: underline; + } } .merge-param-checkbox { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 6e63bb75e88..57cf8e136e2 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -410,8 +410,22 @@ ul.notes { } .discussion-toggle-button { + padding: 0; + background-color: transparent; + border: 0; line-height: 20px; font-size: 13px; + transition: color 0.1s linear; + + &:hover { + color: $gl-link-color; + } + + &:focus { + text-decoration: underline; + outline: none; + color: $gl-link-color; + } .fa { margin-right: 3px; diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 6f5d4bf2a2f..2d78c55211e 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -8,7 +8,7 @@ .discussion.js-toggle-container{ class: discussion.id, data: { discussion_id: discussion.id } } .discussion-header .discussion-actions - = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do + %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button" } - if expanded = icon("chevron-up") - else diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 6ab9a80e083..4b1ff75541a 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -24,7 +24,7 @@ .visible-xs-inline = render_commit_status(commit, ref: ref) - if commit.description? - %a.text-expander.hidden-xs.js-toggle-button ... + %button.text-expander.hidden-xs.js-toggle-button{ type: "button" } ... - if commit.description? %pre.commit-row-description.js-toggle-content diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml index 8e24e28765f..fd4f3c8d3cc 100644 --- a/app/views/projects/diffs/_stats.html.haml +++ b/app/views/projects/diffs/_stats.html.haml @@ -1,7 +1,7 @@ .js-toggle-container .commit-stat-summary Showing - = link_to '#', class: 'js-toggle-button' do + %button.diff-stats-summary-toggler.js-toggle-button{ type: "button" } %strong= pluralize(diff_files.size, "changed file") with %strong.cgreen #{diff_files.sum(&:added_lines)} additions diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index c94c7944c0b..e5ec151a61d 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -37,7 +37,7 @@ = check_box_tag :should_remove_source_branch Remove source branch .accept-control - = link_to "#", class: "modify-merge-commit-link js-toggle-button" do + %button.modify-merge-commit-link.js-toggle-button{ type: "button" } = icon('edit') Modify commit message .js-toggle-content.hide.prepend-top-default diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 34a1214a350..09ac1fd6794 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -76,7 +76,7 @@ Gitea %div - if git_import_enabled? - = link_to "#", class: 'btn js-toggle-button import_git' do + %button.btn.js-toggle-button.import_git{ type: "button" } = icon('git', text: 'Repo by URL') .import_gitlab_project - if gitlab_project_import_enabled? diff --git a/changelogs/unreleased/29414-fix-toggle-discussion-link-jump.yml b/changelogs/unreleased/29414-fix-toggle-discussion-link-jump.yml new file mode 100644 index 00000000000..04342f5359d --- /dev/null +++ b/changelogs/unreleased/29414-fix-toggle-discussion-link-jump.yml @@ -0,0 +1,4 @@ +--- +title: Update toggle buttons to be <button> +merge_request: +author: diff --git a/features/steps/dashboard/new_project.rb b/features/steps/dashboard/new_project.rb index cb36d6ae1a9..d4a04f693b8 100644 --- a/features/steps/dashboard/new_project.rb +++ b/features/steps/dashboard/new_project.rb @@ -19,7 +19,7 @@ class Spinach::Features::NewProject < Spinach::FeatureSteps expect(page).to have_link('Bitbucket') expect(page).to have_link('GitLab.com') expect(page).to have_link('Google Code') - expect(page).to have_link('Repo by URL') + expect(page).to have_button('Repo by URL') expect(page).to have_link('GitLab export') end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 9f0057cace7..c9c4f537fad 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -382,7 +382,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I modify merge commit message' do - find('.modify-merge-commit-link').click + click_button "Modify commit message" fill_in 'commit_message', with: 'wow such merge' end diff --git a/spec/features/merge_requests/diff_notes_resolve_spec.rb b/spec/features/merge_requests/diff_notes_resolve_spec.rb index d5e3d8e7eff..69164aabdb2 100644 --- a/spec/features/merge_requests/diff_notes_resolve_spec.rb +++ b/spec/features/merge_requests/diff_notes_resolve_spec.rb @@ -296,7 +296,7 @@ feature 'Diff notes resolve', feature: true, js: true do it 'displays next discussion even if hidden' do page.all('.note-discussion').each do |discussion| page.within discussion do - click_link 'Toggle discussion' + click_button 'Toggle discussion' end end @@ -477,13 +477,13 @@ feature 'Diff notes resolve', feature: true, js: true do it 'shows resolved icon' do expect(page).to have_content '1/1 discussion resolved' - click_link 'Toggle discussion' + click_button 'Toggle discussion' expect(page).to have_selector('.line-resolve-btn.is-active') end it 'does not allow user to click resolve button' do expect(page).to have_selector('.line-resolve-btn.is-disabled') - click_link 'Toggle discussion' + click_button 'Toggle discussion' expect(page).to have_selector('.line-resolve-btn.is-disabled') end diff --git a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb index 3dbe26cddb0..1bc2a5548dd 100644 --- a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb +++ b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb @@ -41,7 +41,7 @@ feature 'Clicking toggle commit message link', feature: true, js: true do visit namespace_project_merge_request_path(project.namespace, project, merge_request) expect(textbox).not_to be_visible - click_link "Modify commit message" + click_button "Modify commit message" expect(textbox).to be_visible end |