Commit 410130b8 authored by Jacob Schatz's avatar Jacob Schatz

Merge branch '18026-consider-removing-issue-merge-request-previous-next-buttons' into 'master'

Remove prev/next buttons on issues and merge requests

## What does this MR do?

Remove prev/next buttons on issues and merge requests sidebar

## Are there points in the code the reviewer needs to double check?

No

## Why was this MR needed?

The buttons were rarely used and added at least 1 query each on every page load which we could live without.

## What are the relevant issue numbers?

#18026

## Screenshots (if relevant)

Below is how it looks. Not sure if it's too much empty space. #17697 discusses a possible substitute for what to put there.

![Captura_de_pantalla_2016-06-06_a_las_11.09.26_a.m.](/uploads/7b379327aa6c608775f99bf47a9c3352/Captura_de_pantalla_2016-06-06_a_las_11.09.26_a.m..png)

![Captura_de_pantalla_2016-06-06_a_las_11.09.38_a.m.](/uploads/68bcae6754ed5ac48f1a6925fdd17d3f/Captura_de_pantalla_2016-06-06_a_las_11.09.38_a.m..png)

See merge request !4442
parents ed9b1bd6 535d1130
...@@ -46,6 +46,7 @@ v 8.8.4 (unreleased) ...@@ -46,6 +46,7 @@ v 8.8.4 (unreleased)
- Fix todos page throwing errors when you have a project pending deletion - Fix todos page throwing errors when you have a project pending deletion
- Reduce number of SQL queries when rendering user references - Reduce number of SQL queries when rendering user references
- Upgrade to jQuery 2 - Upgrade to jQuery 2
- Remove prev/next buttons on issues and merge requests
v 8.8.3 v 8.8.3
- Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312 - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312
......
...@@ -10,14 +10,6 @@ class @ShortcutsIssuable extends ShortcutsNavigation ...@@ -10,14 +10,6 @@ class @ShortcutsIssuable extends ShortcutsNavigation
@replyWithSelectedText() @replyWithSelectedText()
return false return false
) )
Mousetrap.bind('j', =>
@prevIssue()
return false
)
Mousetrap.bind('k', =>
@nextIssue()
return false
)
Mousetrap.bind('e', => Mousetrap.bind('e', =>
@editIssue() @editIssue()
return false return false
...@@ -29,16 +21,6 @@ class @ShortcutsIssuable extends ShortcutsNavigation ...@@ -29,16 +21,6 @@ class @ShortcutsIssuable extends ShortcutsNavigation
else else
@enabledHelp.push('.hidden-shortcut.issues') @enabledHelp.push('.hidden-shortcut.issues')
prevIssue: ->
$prevBtn = $('.prev-btn')
if not $prevBtn.hasClass('disabled')
Turbolinks.visit($prevBtn.attr('href'))
nextIssue: ->
$nextBtn = $('.next-btn')
if not $nextBtn.hasClass('disabled')
Turbolinks.visit($nextBtn.attr('href'))
replyWithSelectedText: -> replyWithSelectedText: ->
if window.getSelection if window.getSelection
selected = window.getSelection().toString() selected = window.getSelection().toString()
......
...@@ -8,14 +8,6 @@ module IssuablesHelper ...@@ -8,14 +8,6 @@ module IssuablesHelper
"right-sidebar-#{sidebar_gutter_collapsed? ? 'collapsed' : 'expanded'}" "right-sidebar-#{sidebar_gutter_collapsed? ? 'collapsed' : 'expanded'}"
end end
def issuables_count(issuable)
base_issuable_scope(issuable).maximum(:iid)
end
def next_issuable_for(issuable)
base_issuable_scope(issuable).where('iid > ?', issuable.iid).last
end
def multi_label_name(current_labels, default_label) def multi_label_name(current_labels, default_label)
# current_labels may be a string from before # current_labels may be a string from before
if current_labels.is_a?(Array) if current_labels.is_a?(Array)
...@@ -45,10 +37,6 @@ module IssuablesHelper ...@@ -45,10 +37,6 @@ module IssuablesHelper
end end
end end
def prev_issuable_for(issuable)
base_issuable_scope(issuable).where('iid < ?', issuable.iid).first
end
def user_dropdown_label(user_id, default_label) def user_dropdown_label(user_id, default_label)
return default_label if user_id.nil? return default_label if user_id.nil?
return "Unassigned" if user_id == "0" return "Unassigned" if user_id == "0"
......
...@@ -2,23 +2,8 @@ ...@@ -2,23 +2,8 @@
.issuable-sidebar .issuable-sidebar
- can_edit_issuable = can?(current_user, :"admin_#{issuable.to_ability_name}", @project) - can_edit_issuable = can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
.block.issuable-sidebar-header .block.issuable-sidebar-header
%span.issuable-count.hide-collapsed.pull-left
= issuable.iid
of
= issuables_count(issuable)
%a.gutter-toggle.pull-right.js-sidebar-toggle{href: '#'} %a.gutter-toggle.pull-right.js-sidebar-toggle{href: '#'}
= sidebar_gutter_toggle_icon = sidebar_gutter_toggle_icon
.issuable-nav.hide-collapsed.pull-right.btn-group{role: 'group', "aria-label" => '...'}
- if prev_issuable = prev_issuable_for(issuable)
= link_to 'Prev', [@project.namespace.becomes(Namespace), @project, prev_issuable], class: 'btn btn-default prev-btn issuable-pager'
- else
%a.btn.btn-default.issuable-pager.disabled{href: '#'}
Prev
- if next_issuable = next_issuable_for(issuable)
= link_to 'Next', [@project.namespace.becomes(Namespace), @project, next_issuable], class: 'btn btn-default next-btn issuable-pager'
- else
%a.btn.btn-default.issuable-pager.disabled{href: '#'}
Next
= form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, format: :json, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, format: :json, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f|
.block.assignee .block.assignee
......
...@@ -25,13 +25,6 @@ Feature: Project Issues ...@@ -25,13 +25,6 @@ Feature: Project Issues
Scenario: I visit issue page Scenario: I visit issue page
Given I click link "Release 0.4" Given I click link "Release 0.4"
Then I should see issue "Release 0.4" Then I should see issue "Release 0.4"
And I should see "1 of 2" in the sidebar
Scenario: I navigate between issues
Given I click link "Release 0.4"
Then I click link "Next" in the sidebar
Then I should see issue "Tweet control"
And I should see "2 of 2" in the sidebar
@javascript @javascript
Scenario: I filter by author Scenario: I filter by author
......
...@@ -49,14 +49,12 @@ Feature: Project Merge Requests ...@@ -49,14 +49,12 @@ Feature: Project Merge Requests
Scenario: I visit an open merge request page Scenario: I visit an open merge request page
Given I click link "Bug NS-04" Given I click link "Bug NS-04"
Then I should see merge request "Bug NS-04" Then I should see merge request "Bug NS-04"
And I should see "1 of 1" in the sidebar
Scenario: I visit a merged merge request page Scenario: I visit a merged merge request page
Given project "Shop" have "Feature NS-05" merged merge request Given project "Shop" have "Feature NS-05" merged merge request
And I click link "Merged" And I click link "Merged"
And I click link "Feature NS-05" And I click link "Feature NS-05"
Then I should see merge request "Feature NS-05" Then I should see merge request "Feature NS-05"
And I should see "3 of 3" in the sidebar
Scenario: I close merge request page Scenario: I close merge request page
Given I click link "Bug NS-04" Given I click link "Bug NS-04"
......
...@@ -138,22 +138,6 @@ module SharedIssuable ...@@ -138,22 +138,6 @@ module SharedIssuable
end end
end end
step 'I should see "1 of 1" in the sidebar' do
expect_sidebar_content('1 of 1')
end
step 'I should see "1 of 2" in the sidebar' do
expect_sidebar_content('1 of 2')
end
step 'I should see "2 of 2" in the sidebar' do
expect_sidebar_content('2 of 2')
end
step 'I should see "3 of 3" in the sidebar' do
expect_sidebar_content('3 of 3')
end
step 'I click link "Next" in the sidebar' do step 'I click link "Next" in the sidebar' do
page.within '.issuable-sidebar' do page.within '.issuable-sidebar' do
click_link 'Next' click_link 'Next'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment