From 31243cc86d1903c261a750a576c625d155af6d48 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 24 Feb 2022 01:02:15 -0700 Subject: [PATCH 1/2] Swap Merge Request tabs to use the Pajamas (Rails) helper --- app/assets/javascripts/merge_request.js | 16 ++- app/assets/javascripts/merge_request_tabs.js | 99 +++++++++++++++++++ .../stylesheets/pages/merge_requests.scss | 31 ++++++ .../creations/_new_submit.html.haml | 62 ++++++------ 4 files changed, 171 insertions(+), 37 deletions(-) diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 244cf1e150a883..28ecd6994f1a34 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -8,7 +8,7 @@ import eventHub from '~/vue_merge_request_widget/event_hub'; import axios from './lib/utils/axios_utils'; import { addDelimiter } from './lib/utils/text_utility'; import { getParameterValues, setUrlParams } from './lib/utils/url_utility'; -import MergeRequestTabs from './merge_request_tabs'; +import MergeRequestTabs, { createTabs } from './merge_request_tabs'; import TaskList from './task_list'; function MergeRequest(opts) { @@ -51,11 +51,17 @@ MergeRequest.prototype.$ = function (selector) { }; MergeRequest.prototype.initTabs = function () { - if (window.mrTabs) { - window.mrTabs.unbindEvents(); - } + const isCreate = document.querySelector('.js-merge-request-new-submit'); - window.mrTabs = new MergeRequestTabs(this.opts); + if (isCreate) { + createTabs({ ...this.opts }); + } else { + if (window.mrTabs) { + window.mrTabs.unbindEvents(); + } + + window.mrTabs = new MergeRequestTabs(this.opts); + } }; MergeRequest.prototype.initMRBtnListeners = function () { diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 5d28c95d8a2f73..871cd6fd1554fd 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -2,12 +2,14 @@ import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import $ from 'jquery'; import Vue from 'vue'; +import { GlTabsBehavior, TAB_SHOWN_EVENT } from '~/tabs'; import { getCookie, parseUrlPathname, isMetaClick, parseBoolean, scrollToElement, + historyPushState, } from '~/lib/utils/common_utils'; import createEventHub from '~/helpers/event_hub_factory'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; @@ -67,6 +69,7 @@ import syntaxHighlight from './syntax_highlight'; // <100ms is typically indistinguishable from "instant" for users, but allows for re-rendering const FAST_DELAY_FOR_RERENDER = 75; +const TEN_MINUTES = 600000; // Store the `location` object, allowing for easier stubbing in tests let { location } = window; @@ -80,6 +83,10 @@ function scrollToContainer(container) { } } +function tabUrl(tab) { + return tab.getAttribute('href'); +} + function computeTopOffset(tabs) { const navbar = document.querySelector('.navbar-gitlab'); const peek = document.getElementById('js-peek'); @@ -544,3 +551,95 @@ export default class MergeRequestTabs { }, 0); } } + +function getActiveTab({ + links = document.querySelectorAll('.js-merge-request-tabs li.nav-item a[aria-controls]'), + url = window.location, +}) { + const urlAction = url.pathname.split('/').pop(); + const indexes = Array.from(links).map((link) => link.getAttribute('aria-controls')); + const actions = { + new: 'commits', + pipelines: 'pipelines', + diffs: 'diffs', + }; + const action = actions[urlAction]; + + return [indexes.indexOf(action), action]; +} + +function stale(checkTime, age = TEN_MINUTES) { + const now = Date.now(); + + return checkTime ? checkTime <= now - age : true; +} + +function specialTabBehavior(action) { + const behaviors = { + pipelines: ({ pipelines, loads }) => { + let response = { pipelines, loads }; + + if (stale(loads.pipelines)) { + destroyPipelines(pipelines); + + response = { + pipelines: mountPipelines(), + loads: { + ...loads, + pipelines: Date.now(), + }, + }; + } + + return response; + }, + diffs: ({ pipelines, loads, tabs, url }) => { + let response = { pipelines, loads }; + + if (stale(loads.diffs)) { + loadDiffs({ + url: parseUrlPathname(url), + sticky: computeTopOffset(tabs), + }); + + response = { + loads: { + ...loads, + diffs: Date.now(), + }, + pipelines, + }; + } + + return response; + }, + }; + + return behaviors[action] || ((args) => ({ ...args })); +} + +// These are the tabs when comparing two branches prior to creating the MR +export function createTabs() { + const tabsEl = document.querySelector('.js-merge-request-tabs'); + const tabLinks = tabsEl.querySelectorAll('li.nav-item a[aria-controls]'); + const [index] = getActiveTab({ links: tabLinks }); + const tab = tabLinks[index]; + const tabs = new GlTabsBehavior(tabsEl); + let loads = {}; + let pipelines; + + tabsEl.addEventListener(TAB_SHOWN_EVENT, (event) => { + const url = new URL(event.target.getAttribute('href'), window.location.origin); + const [idx, activeAction] = getActiveTab({ links: tabLinks, url }); + + ({ pipelines, loads } = specialTabBehavior(activeAction)({ + url: tabUrl(tabLinks[idx]), + pipelines, + loads, + tabs, + })); + historyPushState(url.href); + }); + + tab.click(); +} diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index f95cff012d036e..7f514fcbfa613d 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -214,6 +214,37 @@ $tabs-holder-z-index: 250; } } +main#content-body { + @include gl-relative; +} + +.mr-compare.merge-request { + @media (min-width: map-get($grid-breakpoints, md)) { + @include gl-sticky; + + $base-top-buffer: -1px; // The default positioning loses a border below the tabs + + background-color: $white; + top: calc(#{$header-height} + #{$base-top-buffer}); + z-index: 121 /* beat the diff file headers */; + + .with-performance-bar & { + $base-top-buffer: -1px + $performance-bar-height; + top: calc(#{$header-height} + #{$base-top-buffer}); + } + + .with-system-header & { + $base-top-buffer: -1px + $system-header-height; + top: calc(#{$header-height} + #{$base-top-buffer}); + } + + .with-system-header.with-performance-bar & { + $base-top-buffer: -1px + $system-header-height + $performance-bar-height; + top: calc(#{$header-height} + #{$base-top-buffer}); + } + } +} + .merge-request-tabs-holder, .epic-tabs-holder { top: $header-height; diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml index 253f50d5090522..f2715110569d2d 100644 --- a/app/views/projects/merge_requests/creations/_new_submit.html.haml +++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml @@ -11,41 +11,39 @@ = hidden_field_tag(:nav_source, params[:nav_source]) .mr-compare.merge-request.js-merge-request-new-submit{ 'data-mr-submit-action': "#{j params[:tab].presence || 'new'}" } - - if @commits.empty? - .commits-empty - %h4 - There are no commits yet. - = custom_icon ('illustration_no_commits') - - else - .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - .merge-request-tabs-container - .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller - .fade-left= sprite_icon('chevron-lg-left', size: 12) - .fade-right= sprite_icon('chevron-lg-right', size: 12) - %ul.merge-request-tabs.nav.nav-tabs.nav-links.no-top.no-bottom.js-tabs-affix - %li.commits-tab.new-tab - = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tabvue'} do - Commits - = gl_badge_tag @total_commit_count, { size: :sm }, { class: 'gl-tab-counter-badge' } - - if @pipelines.any? - %li.builds-tab - = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tabvue'} do - Pipelines - = gl_badge_tag @pipelines.size, { size: :sm }, { class: 'gl-tab-counter-badge' } - %li.diffs-tab - = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tabvue', qa_selector: 'diffs_tab'} do - Changes - = gl_badge_tag @merge_request.diff_size, { size: :sm }, { class: 'gl-tab-counter-badge' } + - if !@commits.empty? - #diff-notes-app.tab-content - #new.commits.tab-pane.active - = render "projects/merge_requests/commits" - #diffs.diffs.tab-pane{ class: "gl-m-0!" } - -# This tab is always loaded via AJAX + - commits = url_for(safe_params) + - pipelines = url_for(safe_params.merge(action: 'pipelines')) + - diffs = url_for(safe_params.merge(action: 'diffs')) + + = gl_tabs_nav({ class: "js-merge-request-tabs" }) do + = gl_tab_link_to commits, item_active: true, 'aria-controls': 'commits' do + = _('Commits') + = gl_tab_counter_badge @total_commit_count - if @pipelines.any? - #pipelines.pipelines.tab-pane - = render 'projects/merge_requests/pipelines', endpoint: url_for(safe_params.merge(action: 'pipelines', format: :json)), disable_initialization: true + = gl_tab_link_to pipelines, 'aria-controls': 'pipelines' do + = _('Pipelines') + = gl_tab_counter_badge @pipelines.size + = gl_tab_link_to diffs, 'aria-controls': 'diffs' do + = _('Changes') + = gl_tab_counter_badge @merge_request.diff_size .mr-loading-status .loading.hide = gl_loading_icon(size: 'md') + +#diff-notes-app.tab-content + #commits.commits.tab-pane.active + - if @commits.empty? + .commits-empty + %h4= _('There are no commits yet.') + = custom_icon ('illustration_no_commits') + - else + = render "projects/merge_requests/commits" + - if @pipelines.any? + #pipelines.pipelines.tab-pane + = render 'projects/merge_requests/pipelines', endpoint: url_for(safe_params.merge(action: 'pipelines', format: :json)), disable_initialization: true + #diffs.diffs.tab-pane{ class: "gl-m-0!" } + -# This tab is always loaded via AJAX + -- GitLab From dc7c809e2b0cb2855ef427914ba86c1fdda56d43 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Fri, 11 Mar 2022 15:51:45 -0700 Subject: [PATCH 2/2] cause test failures??? --- app/assets/javascripts/merge_request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 28ecd6994f1a34..0cbbb38de6d3ad 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -54,7 +54,7 @@ MergeRequest.prototype.initTabs = function () { const isCreate = document.querySelector('.js-merge-request-new-submit'); if (isCreate) { - createTabs({ ...this.opts }); + // createTabs({ ...this.opts }); } else { if (window.mrTabs) { window.mrTabs.unbindEvents(); -- GitLab