From 4e03f4c40602b568cffd591dcd5af6bd4b9a281e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 19 Oct 2016 14:57:42 +0100 Subject: [PATCH 1/2] Fixed issues with sticky mr tabs & sidebar Closes #23504 --- app/assets/javascripts/merge_request_tabs.js | 11 +-- app/assets/stylesheets/framework/sidebar.scss | 8 +++ .../stylesheets/pages/merge_requests.scss | 13 +++- .../projects/merge_requests/_show.html.haml | 68 ++++++++++--------- 4 files changed, 54 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index fd21aa1fefa..1a04a037210 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -388,8 +388,7 @@ // So we dont affix the tabs on these if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return; - var tabsWidth = $tabs.outerWidth(), - $diffTabs = $('#diff-notes-app'), + var $diffTabs = $('#diff-notes-app'), offsetTop = $tabs.offset().top - ($('.navbar-fixed-top').height() + $('.layout-nav').height()); $tabs.off('affix.bs.affix affix-top.bs.affix') @@ -398,18 +397,10 @@ top: offsetTop } }).on('affix.bs.affix', function () { - $tabs.css({ - left: $tabs.offset().left, - width: tabsWidth - }); $diffTabs.css({ marginTop: $tabs.height() }); }).on('affix-top.bs.affix', function () { - $tabs.css({ - left: '', - width: '' - }); $diffTabs.css({ marginTop: '' }); diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index ec52f326eb9..1d8e64a0e4b 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -185,6 +185,10 @@ header.header-sidebar-pinned { @media (min-width: $screen-sm-min) { padding-right: $sidebar_collapsed_width; + + .merge-request-tabs-holder.affix { + right: $sidebar_collapsed_width; + } } .sidebar-collapsed-icon { @@ -207,6 +211,10 @@ header.header-sidebar-pinned { @media (min-width: $screen-md-min) { padding-right: $gutter_width; + + .merge-request-tabs-holder.affix { + right: $gutter_width; + } } &.with-overlay { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 35a1877df95..70afa568554 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -183,11 +183,11 @@ .ci-coverage { float: right; } - + .stop-env-container { color: $gl-text-color; float: right; - + a { color: $gl-text-color; } @@ -438,11 +438,18 @@ } } -.merge-request-tabs { +.merge-request-tabs-holder { background-color: #fff; &.affix { top: 100px; + left: 0; z-index: 9; + transition: right .15s; + } + + &:not(.affix) .container-fluid { + padding-left: 0; + padding-right: 0; } } diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 0e19d224fcd..f57abe73977 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -47,39 +47,41 @@ = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal" - if @commits_count.nonzero? - %ul.merge-request-tabs.nav-links.no-top.no-bottom{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - %li.notes-tab - = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do - Discussion - %span.badge= @merge_request.mr_and_commit_notes.user.count - - if @merge_request.source_project - %li.commits-tab - = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do - Commits - %span.badge= @commits_count - - if @pipeline - %li.pipelines-tab - = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do - Pipelines - %span.badge= @pipelines.size - %li.builds-tab - = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do - Builds - %span.badge= @statuses.size - %li.diffs-tab - = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do - Changes - %span.badge= @merge_request.diff_size - %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } - %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } - .line-resolve-all{ "v-show" => "discussionCount > 0", - ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } - %span.line-resolve-btn.is-disabled{ type: "button", - ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } - = render "shared/icons/icon_status_success.svg" - %span.line-resolve-text - {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved - = render "discussions/jump_to_next" + .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } + %div{ class: container_class } + %ul.merge-request-tabs.nav-links.no-top.no-bottom + %li.notes-tab + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do + Discussion + %span.badge= @merge_request.mr_and_commit_notes.user.count + - if @merge_request.source_project + %li.commits-tab + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do + Commits + %span.badge= @commits_count + - if @pipeline + %li.pipelines-tab + = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do + Pipelines + %span.badge= @pipelines.size + %li.builds-tab + = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do + Builds + %span.badge= @statuses.size + %li.diffs-tab + = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do + Changes + %span.badge= @merge_request.diff_size + %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + .line-resolve-all{ "v-show" => "discussionCount > 0", + ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } + %span.line-resolve-btn.is-disabled{ type: "button", + ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } + = render "shared/icons/icon_status_success.svg" + %span.line-resolve-text + {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved + = render "discussions/jump_to_next" .tab-content#diff-notes-app #notes.notes.tab-pane.voting_notes From a28371dbe33c568c970c704b90760d2b540256af Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 19 Oct 2016 17:24:24 +0100 Subject: [PATCH 2/2] Fixed issue when images are loading it would push off the tabs --- app/assets/javascripts/merge_request_tabs.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 1a04a037210..9f28738e06b 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -389,12 +389,18 @@ if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return; var $diffTabs = $('#diff-notes-app'), - offsetTop = $tabs.offset().top - ($('.navbar-fixed-top').height() + $('.layout-nav').height()); + $fixedNav = $('.navbar-fixed-top'), + $layoutNav = $('.layout-nav'); $tabs.off('affix.bs.affix affix-top.bs.affix') .affix({ offset: { - top: offsetTop + top: function () { + var tabsTop = $diffTabs.offset().top - $tabs.height(); + tabsTop = tabsTop - ($fixedNav.height() + $layoutNav.height()); + + return tabsTop; + } } }).on('affix.bs.affix', function () { $diffTabs.css({