From ab8d36452d1b7004a4108b80af8beca115e3a15a Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Wed, 19 Apr 2017 11:56:24 +1000 Subject: fix Resolved Discussions counter wrapping to next line change all .nav-links to use flex place Resolve Discussions above tabs on small screens, and to the right on large --- app/assets/stylesheets/framework/nav.scss | 22 +++---- app/assets/stylesheets/pages/issuable.scss | 10 ---- app/assets/stylesheets/pages/merge_requests.scss | 18 ++++++ app/assets/stylesheets/pages/notes.scss | 7 +++ app/views/projects/merge_requests/_show.html.haml | 70 +++++++++++----------- changelogs/unreleased/31106-tabs-alignment.yml | 4 ++ ..._issue_for_discussions_in_merge_request_spec.rb | 2 +- 7 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 changelogs/unreleased/31106-tabs-alignment.yml diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index e6d808717f3..27ae487083c 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -24,10 +24,10 @@ } @mixin scrolling-links() { - white-space: nowrap; overflow-x: auto; overflow-y: hidden; -webkit-overflow-scrolling: touch; + display: flex; &::-webkit-scrollbar { display: none; @@ -35,6 +35,7 @@ } .nav-links { + display: flex; padding: 0; margin: 0; list-style: none; @@ -42,17 +43,16 @@ border-bottom: 1px solid $border-color; li { - display: inline-block; + display: flex; a { - display: inline-block; padding: $gl-btn-padding; padding-bottom: 11px; - margin-bottom: -1px; font-size: 14px; line-height: 28px; color: $gl-text-color-secondary; border-bottom: 2px solid transparent; + white-space: nowrap; &:hover, &:active, @@ -85,10 +85,10 @@ .container-fluid { background-color: $gray-normal; margin-bottom: 0; + display: flex; } li { - &.active a { border-bottom: none; color: $link-underline-blue; @@ -137,9 +137,9 @@ } .nav-links { - display: inline-block; margin-bottom: 0; border-bottom: none; + float: left; &.wide { width: 100%; @@ -336,6 +336,10 @@ border-bottom: none; height: 51px; + @media (min-width: $screen-sm-min) { + justify-content: center; + } + li { a { padding-top: 10px; @@ -346,6 +350,7 @@ .scrolling-tabs-container { position: relative; + overflow: hidden; .nav-links { @include scrolling-links(); @@ -483,10 +488,7 @@ .inner-page-scroll-tabs { position: relative; - - .nav-links { - padding-bottom: 1px; - } + overflow: hidden; .fade-right { @include fade(left, $white-light); diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 0bca3e93e4c..dc67d8a4f29 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -17,16 +17,6 @@ .merge-manually { @extend .fixed-width-container; } - - .merge-request-tabs-holder { - &.affix { - border-bottom: 1px solid $border-color; - - .nav-links { - border: 0; - } - } - } } .merge-request-details { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 6a419384a34..2393b892757 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -551,6 +551,7 @@ .merge-request-tabs-holder { background-color: $white-light; + border-bottom: 1px solid $border-color; &.affix { top: 0; @@ -567,6 +568,16 @@ padding-right: $gl-padding; } } + + .nav-links { + border: 0; + } +} + +.merge-request-tabs { + display: flex; + margin-bottom: 0; + padding: 0; } .limit-container-width { @@ -577,6 +588,13 @@ } } +.merge-request-tabs-container { + display: flex; + flex-wrap: wrap; + flex-direction: row-reverse; + justify-content: flex-end; +} + .limit-container-width:not(.container-limited) { .merge-request-tabs-holder:not(.affix) { .merge-request-tabs-container { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 2ea2ff8362b..75cc3d32990 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -584,6 +584,13 @@ ul.notes { } .line-resolve-all-container { + margin-left: auto; + padding-left: $gl-padding; + + > div { + white-space: nowrap; + } + .btn-group { margin-left: -4px; } diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 881ee9fd596..6c106f3a7a5 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -52,40 +52,42 @@ = render 'award_emoji/awards_block', awardable: @merge_request, inline: true .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - .merge-request-tabs-container.scrolling-tabs-container.inner-page-scroll-tabs - .fade-left= icon('angle-left') - .fade-right= icon('angle-right') - %ul.merge-request-tabs.nav-links.scrolling-tabs - %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.related_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 @pipelines.any? - %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.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?}" } - %div - .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 }} {{ resolvedCountText }} resolved - = render "discussions/new_issue_for_all_discussions", merge_request: @merge_request - = render "discussions/jump_to_next" + .merge-request-tabs-container + #resolve-count-app.line-resolve-all-container.prepend-top-10{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + %div + .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 }} {{ resolvedCountText }} resolved + = render "discussions/new_issue_for_all_discussions", merge_request: @merge_request + = render "discussions/jump_to_next" + .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller + .fade-left= icon('angle-left') + .fade-right= icon('angle-right') + .nav-links.scrolling-tabs + %ul.merge-request-tabs + %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.related_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 @pipelines.any? + %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.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 .tab-content#diff-notes-app #notes.notes.tab-pane.voting_notes diff --git a/changelogs/unreleased/31106-tabs-alignment.yml b/changelogs/unreleased/31106-tabs-alignment.yml new file mode 100644 index 00000000000..53da08cc32d --- /dev/null +++ b/changelogs/unreleased/31106-tabs-alignment.yml @@ -0,0 +1,4 @@ +--- +title: prevent nav tabs from wrapping to new line +merge_request: +author: diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index 58f897cba3e..01fea762635 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -14,7 +14,7 @@ feature 'Resolving all open discussions in a merge request from an issue', featu end it 'shows a button to resolve all discussions by creating a new issue' do - within('li#resolve-count-app') do + within('#resolve-count-app') do expect(page).to have_link "Resolve all discussions in new issue", href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) end end -- cgit v1.2.1