Commit f8e27b92bfe12bb3bf6e98bcaa9078941eaf7823
Exists in
master
and in
4 other branches
Merge pull request #1902 from tsigo/breadcrumbs
Fix breadcrumb links on Commits page
Showing
7 changed files
with
63 additions
and
30 deletions
Show diff stats
app/assets/javascripts/tree.js.coffee
| @@ -17,23 +17,21 @@ $ -> | @@ -17,23 +17,21 @@ $ -> | ||
| 17 | "ajax:beforeSend": -> $('.tree_progress').addClass("loading") | 17 | "ajax:beforeSend": -> $('.tree_progress').addClass("loading") |
| 18 | "ajax:complete": -> $('.tree_progress').removeClass("loading") | 18 | "ajax:complete": -> $('.tree_progress').removeClass("loading") |
| 19 | 19 | ||
| 20 | -# Maintain forward/back history while browsing the file tree | ||
| 21 | - | ||
| 22 | -((window) -> | ||
| 23 | - History = window.History | ||
| 24 | - $ = window.jQuery | ||
| 25 | - document = window.document | ||
| 26 | - | ||
| 27 | - # Check to see if History.js is enabled for our Browser | ||
| 28 | - unless History.enabled | ||
| 29 | - return false | ||
| 30 | - | ||
| 31 | - $ -> | ||
| 32 | - $('#tree-slider .tree-item-file-name a, .breadcrumb li > a').live 'click', (e) -> | ||
| 33 | - History.pushState(null, null, $(@).attr('href')) | ||
| 34 | - return false | ||
| 35 | - | ||
| 36 | - History.Adapter.bind window, 'statechange', -> | ||
| 37 | - state = History.getState() | ||
| 38 | - window.ajaxGet(state.url) | ||
| 39 | -)(window) | 20 | + # Maintain forward/back history while browsing the file tree |
| 21 | + ((window) -> | ||
| 22 | + History = window.History | ||
| 23 | + $ = window.jQuery | ||
| 24 | + document = window.document | ||
| 25 | + | ||
| 26 | + # Check to see if History.js is enabled for our Browser | ||
| 27 | + unless History.enabled | ||
| 28 | + return false | ||
| 29 | + | ||
| 30 | + $('#tree-slider .tree-item-file-name a, .breadcrumb li > a').live 'click', (e) -> | ||
| 31 | + History.pushState(null, null, $(@).attr('href')) | ||
| 32 | + return false | ||
| 33 | + | ||
| 34 | + History.Adapter.bind window, 'statechange', -> | ||
| 35 | + state = History.getState() | ||
| 36 | + window.ajaxGet(state.url) | ||
| 37 | + )(window) |
app/decorators/tree_decorator.rb
| @@ -8,14 +8,14 @@ class TreeDecorator < ApplicationDecorator | @@ -8,14 +8,14 @@ class TreeDecorator < ApplicationDecorator | ||
| 8 | 8 | ||
| 9 | #parts = parts[0...-1] if is_blob? | 9 | #parts = parts[0...-1] if is_blob? |
| 10 | 10 | ||
| 11 | - yield(h.link_to("..", "#", remote: true)) if parts.count > max_links | 11 | + yield(h.link_to("..", "#")) if parts.count > max_links |
| 12 | 12 | ||
| 13 | parts.each do |part| | 13 | parts.each do |part| |
| 14 | part_path = File.join(part_path, part) unless part_path.empty? | 14 | part_path = File.join(part_path, part) unless part_path.empty? |
| 15 | part_path = part if part_path.empty? | 15 | part_path = part if part_path.empty? |
| 16 | 16 | ||
| 17 | next unless parts.last(2).include?(part) if parts.count > max_links | 17 | next unless parts.last(2).include?(part) if parts.count > max_links |
| 18 | - yield(h.link_to(h.truncate(part, length: 40), h.project_tree_path(project, h.tree_join(ref, part_path)), remote: true)) | 18 | + yield(h.link_to(h.truncate(part, length: 40), h.project_tree_path(project, h.tree_join(ref, part_path)))) |
| 19 | end | 19 | end |
| 20 | end | 20 | end |
| 21 | end | 21 | end |
app/helpers/tree_helper.rb
| @@ -67,4 +67,29 @@ module TreeHelper | @@ -67,4 +67,29 @@ module TreeHelper | ||
| 67 | can?(current_user, :push_code, @project) | 67 | can?(current_user, :push_code, @project) |
| 68 | end | 68 | end |
| 69 | end | 69 | end |
| 70 | + | ||
| 71 | + # Breadcrumb links for a Project and, if applicable, a tree path | ||
| 72 | + def breadcrumbs | ||
| 73 | + return unless @project && @ref | ||
| 74 | + | ||
| 75 | + # Add the root project link and the arrow icon | ||
| 76 | + crumbs = content_tag(:li) do | ||
| 77 | + content_tag(:span, nil, class: 'arrow') + | ||
| 78 | + link_to(@project.name, project_commits_path(@project, @ref)) | ||
| 79 | + end | ||
| 80 | + | ||
| 81 | + if @path | ||
| 82 | + parts = @path.split('/') | ||
| 83 | + | ||
| 84 | + parts.each_with_index do |part, i| | ||
| 85 | + crumbs += content_tag(:span, '/', class: 'divider') | ||
| 86 | + crumbs += content_tag(:li) do | ||
| 87 | + # The text is just the individual part, but the link needs all the parts before it | ||
| 88 | + link_to part, project_commits_path(@project, tree_join(@ref, parts[0..i].join('/'))) | ||
| 89 | + end | ||
| 90 | + end | ||
| 91 | + end | ||
| 92 | + | ||
| 93 | + crumbs.html_safe | ||
| 94 | + end | ||
| 70 | end | 95 | end |
app/views/commits/show.html.haml
| @@ -2,14 +2,7 @@ | @@ -2,14 +2,7 @@ | ||
| 2 | 2 | ||
| 3 | - if @path.present? | 3 | - if @path.present? |
| 4 | %ul.breadcrumb | 4 | %ul.breadcrumb |
| 5 | - %li | ||
| 6 | - %span.arrow | ||
| 7 | - = link_to project_commits_path(@project) do | ||
| 8 | - = @project.name | ||
| 9 | - %span.divider | ||
| 10 | - \/ | ||
| 11 | - %li | ||
| 12 | - %a{href: "#"}= @path.split("/").join(" / ") | 5 | + = breadcrumbs |
| 13 | 6 | ||
| 14 | %div{id: dom_id(@project)} | 7 | %div{id: dom_id(@project)} |
| 15 | #commits_list= render "commits" | 8 | #commits_list= render "commits" |
features/project/commits/commits.feature
| @@ -19,3 +19,7 @@ Feature: Project Browse commits | @@ -19,3 +19,7 @@ Feature: Project Browse commits | ||
| 19 | Given I visit compare refs page | 19 | Given I visit compare refs page |
| 20 | And I fill compare fields with refs | 20 | And I fill compare fields with refs |
| 21 | Then I see compared refs | 21 | Then I see compared refs |
| 22 | + | ||
| 23 | + Scenario: I browse commits for a specific path | ||
| 24 | + Given I visit my project's commits page for a specific path | ||
| 25 | + Then I see breadcrumb links |
features/steps/project/project_browse_commits.rb
| @@ -42,4 +42,13 @@ class ProjectBrowseCommits < Spinach::FeatureSteps | @@ -42,4 +42,13 @@ class ProjectBrowseCommits < Spinach::FeatureSteps | ||
| 42 | page.should have_content "Commits (1)" | 42 | page.should have_content "Commits (1)" |
| 43 | page.should have_content "Showing 2 changed files" | 43 | page.should have_content "Showing 2 changed files" |
| 44 | end | 44 | end |
| 45 | + | ||
| 46 | + Then 'I see breadcrumb links' do | ||
| 47 | + page.should have_selector('ul.breadcrumb') | ||
| 48 | + page.should have_selector('ul.breadcrumb span.divider', count: 3) | ||
| 49 | + page.should have_selector('ul.breadcrumb a', count: 4) | ||
| 50 | + | ||
| 51 | + find('ul.breadcrumb li:first a')['href'].should match(/#{@project.path}\/commits\/master\z/) | ||
| 52 | + find('ul.breadcrumb li:last a')['href'].should match(%r{master/app/models/project\.rb\z}) | ||
| 53 | + end | ||
| 45 | end | 54 | end |
features/steps/shared/paths.rb
| @@ -121,6 +121,10 @@ module SharedPaths | @@ -121,6 +121,10 @@ module SharedPaths | ||
| 121 | visit project_commits_path(@project, @project.root_ref, {limit: 5}) | 121 | visit project_commits_path(@project, @project.root_ref, {limit: 5}) |
| 122 | end | 122 | end |
| 123 | 123 | ||
| 124 | + Given "I visit my project's commits page for a specific path" do | ||
| 125 | + visit project_commits_path(@project, @project.root_ref + "/app/models/project.rb", {limit: 5}) | ||
| 126 | + end | ||
| 127 | + | ||
| 124 | Given "I visit my project's network page" do | 128 | Given "I visit my project's network page" do |
| 125 | # Stub GraphCommit max_size to speed up test (10 commits vs. 650) | 129 | # Stub GraphCommit max_size to speed up test (10 commits vs. 650) |
| 126 | Gitlab::GraphCommit.stub(max_count: 10) | 130 | Gitlab::GraphCommit.stub(max_count: 10) |