ensure the 'fixed layout' preference is honored whenever possible

see #22343 for issue description

CHANGELOG

@@ -27,6 +27,7 @@ v 8.12.0 (unreleased)
   - Use Search::GlobalService.new in the `GET /projects/search/:query` endpoint
   - Fix long comments in diffs messing with table width
   - Fix pagination on user snippets page
+  - Honor "fixed layout" preference in more places !6422
   - Run CI builds with the permissions of users !5735
   - Fix sorting of issues in API
   - Sort project variables by key. !6275 (Diego Souza)

app/assets/javascripts/diff.js

@@ -7,6 +7,9 @@
     function Diff() {
       $('.files .diff-file').singleFileDiff();
       this.filesCommentButton = $('.files .diff-file').filesCommentButton();
+      if (this.diffViewType() === 'parallel') {
+        $('.content-wrapper .container-fluid').removeClass('container-limited');
+      }
       $(document).off('click', '.js-unfold');
       $(document).on('click', '.js-unfold', (function(_this) {
         return function(event) {
@@ -52,6 +55,10 @@
       })(this));
     }
 
+    Diff.prototype.diffViewType = function() {
+      return $('.inline-parallel-buttons a.active').data('view-type');
+    }
+
     Diff.prototype.lineNumbers = function(line) {
       if (!line.children().length) {
         return [0, 0];

app/assets/javascripts/merge_conflict_data_provider.js.es6

@@ -7,13 +7,16 @@ const ORIGIN_BUTTON_TITLE = 'Use theirs';
 class MergeConflictDataProvider {
 
   getInitialData() {
+    // TODO: remove reliance on jQuery and DOM state introspection
     const diffViewType = $.cookie('diff_view');
+    const fixedLayout = $('.content-wrapper .container-fluid').hasClass('container-limited');
 
     return {
       isLoading      : true,
       hasError       : false,
       isParallel     : diffViewType === 'parallel',
       diffViewType   : diffViewType,
+      fixedLayout    : fixedLayout,
       isSubmitting   : false,
       conflictsData  : {},
       resolutionData : {}
@@ -192,14 +195,15 @@ class MergeConflictDataProvider {
   updateViewType(newType) {
     const vi = this.vueInstance;
 
-    if (newType === vi.diffView || !(newType === 'parallel' || newType === 'inline')) {
+    if (newType === vi.diffViewType || !(newType === 'parallel' || newType === 'inline')) {
       return;
     }
 
-    vi.diffView   = newType;
+    vi.diffViewType = newType;
     vi.isParallel   = newType === 'parallel';
     $.cookie('diff_view', newType); // TODO: Make sure that cookie path added.
-    $('.content-wrapper .container-fluid').toggleClass('container-limited');
+    $('.content-wrapper .container-fluid')
+      .toggleClass('container-limited', !vi.isParallel && vi.fixedLayout);
   }
 
 

app/assets/javascripts/merge_conflict_resolver.js.es6

@@ -60,9 +60,8 @@ class MergeConflictResolver {
           $('#conflicts .js-syntax-highlight').syntaxHighlight();
         });
 
-        if (this.vue.diffViewType === 'parallel') {
-          $('.content-wrapper .container-fluid').removeClass('container-limited');
-        }
+        $('.content-wrapper .container-fluid')
+          .toggleClass('container-limited', !this.vue.isParallel && this.vue.fixedLayout);
       })
   }
 

app/assets/javascripts/merge_request.js

@@ -36,13 +36,10 @@
     };
 
     MergeRequest.prototype.initTabs = function() {
-      if (this.opts.action !== 'new') {
-        // `MergeRequests#new` has no tab-persisting or lazy-loading behavior
-        window.mrTabs = new MergeRequestTabs(this.opts);
-      } else {
-        // Show the first tab (Commits)
-        return $('.merge-request-tabs a[data-toggle="tab"]:first').tab('show');
+      if (window.mrTabs) {
+        window.mrTabs.unbindEvents();
       }
+      window.mrTabs = new MergeRequestTabs(this.opts);
     };
 
     MergeRequest.prototype.showAllCommits = function() {

app/assets/javascripts/merge_request_tabs.js

@@ -56,6 +56,8 @@
 
     MergeRequestTabs.prototype.commitsLoaded = false;
 
+    MergeRequestTabs.prototype.fixedLayoutPref = null;
+
     function MergeRequestTabs(opts) {
       this.opts = opts != null ? opts : {};
       this.opts.setUrl = this.opts.setUrl !== undefined ? this.opts.setUrl : true;
@@ -70,7 +72,12 @@
 
     MergeRequestTabs.prototype.bindEvents = function() {
       $(document).on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
-      return $(document).on('click', '.js-show-tab', this.showTab);
+      $(document).on('click', '.js-show-tab', this.showTab);
+    };
+
+    MergeRequestTabs.prototype.unbindEvents = function() {
+      $(document).off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
+      $(document).off('click', '.js-show-tab', this.showTab);
     };
 
     MergeRequestTabs.prototype.showTab = function(event) {
@@ -85,11 +92,15 @@
       if (action === 'commits') {
         this.loadCommits($target.attr('href'));
         this.expandView();
+        this.resetViewContainer();
       } else if (action === 'diffs') {
         this.loadDiff($target.attr('href'));
         if ((typeof bp !== "undefined" && bp !== null) && bp.getBreakpointSize() !== 'lg') {
           this.shrinkView();
         }
+        if (this.diffViewType() === 'parallel') {
+          this.expandViewContainer();
+        }
         navBarHeight = $('.navbar-gitlab').outerHeight();
         $.scrollTo(".merge-request-details .merge-request-tabs", {
           offset: -navBarHeight
@@ -97,11 +108,14 @@
       } else if (action === 'builds') {
         this.loadBuilds($target.attr('href'));
         this.expandView();
+        this.resetViewContainer();
       } else if (action === 'pipelines') {
         this.loadPipelines($target.attr('href'));
         this.expandView();
+        this.resetViewContainer();
       } else {
         this.expandView();
+        this.resetViewContainer();
       }
       if (this.opts.setUrl) {
         this.setCurrentAction(action);
@@ -126,7 +140,7 @@
       if (action === 'show') {
         action = 'notes';
       }
-      return $(".merge-request-tabs a[data-action='" + action + "']").tab('show');
+      $(".merge-request-tabs a[data-action='" + action + "']").tab('show').trigger('shown.bs.tab');
     };
 
     // Replaces the current Merge Request-specific action in the URL with a new one
@@ -209,7 +223,7 @@
             gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
             $('#diffs .js-syntax-highlight').syntaxHighlight();
             $('#diffs .diff-file').singleFileDiff();
-            if (_this.diffViewType() === 'parallel') {
+            if (_this.diffViewType() === 'parallel' && _this.currentAction === 'diffs') {
               _this.expandViewContainer();
             }
             _this.diffsLoaded = true;
@@ -308,11 +322,21 @@
 
     MergeRequestTabs.prototype.diffViewType = function() {
       return $('.inline-parallel-buttons a.active').data('view-type');
-    // Returns diff view type
     };
 
     MergeRequestTabs.prototype.expandViewContainer = function() {
-      return $('.container-fluid').removeClass('container-limited');
+      var $wrapper = $('.content-wrapper .container-fluid');
+      if (this.fixedLayoutPref === null) {
+        this.fixedLayoutPref = $wrapper.hasClass('container-limited');
+      }
+      $wrapper.removeClass('container-limited');
+    };
+
+    MergeRequestTabs.prototype.resetViewContainer = function() {
+      if (this.fixedLayoutPref !== null) {
+        $('.content-wrapper .container-fluid')
+          .toggleClass('container-limited', this.fixedLayoutPref);
+      }
     };
 
     MergeRequestTabs.prototype.shrinkView = function() {

app/helpers/page_layout_helper.rb

@@ -92,12 +92,8 @@ module PageLayoutHelper
     end
   end
 
-  def fluid_layout(enabled = false)
-    if @fluid_layout.nil?
-      @fluid_layout = (current_user && current_user.layout == "fluid") || enabled
-    else
-      @fluid_layout
-    end
+  def fluid_layout
+    current_user && current_user.layout == "fluid"
   end
 
   def blank_container(enabled = false)

app/views/layouts/header/_default.html.haml

 %header.navbar.navbar-fixed-top.navbar-gitlab{ class: nav_header_class }
-  %div{ class: fluid_layout ? "container-fluid" : "container-fluid" }
+  %div{ class: "container-fluid" }
     .header-content
       %button.side-nav-toggle{ type: 'button', "aria-label" => "Toggle global navigation" }
         %span.sr-only Toggle navigation

app/views/profiles/preferences/update.js.erb

@@ -4,9 +4,9 @@ $('body').addClass('<%= user_application_theme %>')
 
 // Toggle container-fluid class
 if ('<%= current_user.layout %>' === 'fluid') {
-  $('.content-wrapper').find('.container-fluid').removeClass('container-limited')
+  $('.content-wrapper .container-fluid').removeClass('container-limited')
 } else {
-  $('.content-wrapper').find('.container-fluid').addClass('container-limited')
+  $('.content-wrapper .container-fluid').addClass('container-limited')
 }
 
 // Re-enable the "Save" button

app/views/projects/diffs/_diffs.html.haml

 - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true)
 - diff_files = diffs.diff_files
-- if diff_view == :parallel
-  - fluid_layout true
 
 .content-block.oneline-block.files-changed
   .inline-parallel-buttons

app/views/projects/merge_requests/_show.html.haml

@@ -4,9 +4,6 @@
 - content_for :page_specific_javascripts do
   = page_specific_javascript_tag('diff_notes/diff_notes_bundle.js')
 
-- if diff_view == :parallel
-  - fluid_layout true
-
 .merge-request{'data-url' => merge_request_path(@merge_request)}
   = render "projects/merge_requests/show/mr_title"