Merge branch 'ph/341809/removeFluidChangesLayoutFlag' into 'master'

Remove mr_changes_fluid_layout feature flag

See merge request gitlab-org/gitlab!78696

app/assets/javascripts/diffs/components/app.vue

@@ -29,7 +29,6 @@ import {
   MAX_TREE_WIDTH,
   TREE_HIDE_STATS_WIDTH,
   MR_TREE_SHOW_KEY,
-  CENTERED_LIMITED_CONTAINER_CLASSES,
   ALERT_OVERFLOW_HIDDEN,
   ALERT_MERGE_CONFLICT,
   ALERT_COLLAPSED_FILES,
@@ -253,13 +252,6 @@ export default {
     hideFileStats() {
       return this.treeWidth <= TREE_HIDE_STATS_WIDTH;
     },
-    isLimitedContainer() {
-      if (this.glFeatures.mrChangesFluidLayout) {
-        return false;
-      }
-
-      return !this.renderFileTree && !this.isParallelView && !this.isFluidLayout;
-    },
     isFullChangeset() {
       return this.startVersion === null && this.latestDiff;
     },
@@ -395,8 +387,6 @@ export default {
     this.adjustView();
     this.subscribeToEvents();
 
-    this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
-
     this.unwatchDiscussions = this.$watch(
       () => `${this.diffFiles.length}:${this.$store.state.notes.discussions.length}`,
       () => this.setDiscussions(),
@@ -643,10 +633,7 @@ export default {
   <div v-show="shouldShow">
     <div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div>
     <div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane">
-      <compare-versions
-        :is-limited-container="isLimitedContainer"
-        :diff-files-count-text="numTotalFiles"
-      />
+      <compare-versions :diff-files-count-text="numTotalFiles" />
 
       <template v-if="!isBatchLoadingError">
         <hidden-files-warning
@@ -656,10 +643,7 @@ export default {
           :plain-diff-path="plainDiffPath"
           :email-patch-path="emailPatchPath"
         />
-        <collapsed-files-warning
-          v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
-          :limited="isLimitedContainer"
-        />
+        <collapsed-files-warning v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES" />
       </template>
 
       <div
@@ -681,12 +665,7 @@ export default {
           />
           <tree-list :hide-file-stats="hideFileStats" />
         </div>
-        <div
-          class="col-12 col-md-auto diff-files-holder"
-          :class="{
-            [CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer,
-          }"
-        >
+        <div class="col-12 col-md-auto diff-files-holder">
           <commit-widget v-if="commit" :commit="commit" :collapsible="false" />
           <gl-alert
             v-if="isBatchLoadingError"

app/assets/javascripts/diffs/components/collapsed_files_warning.vue

@@ -2,7 +2,7 @@
 import { GlAlert, GlButton } from '@gitlab/ui';
 import { mapState } from 'vuex';
 
-import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants';
+import { EVT_EXPAND_ALL_FILES } from '../constants';
 import eventHub from '../event_hub';
 
 export default {
@@ -11,11 +11,6 @@ export default {
     GlButton,
   },
   props: {
-    limited: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     dismissed: {
       type: Boolean,
       required: false,
@@ -29,11 +24,6 @@ export default {
   },
   computed: {
     ...mapState('diffs', ['diffFiles']),
-    containerClasses() {
-      return {
-        [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited,
-      };
-    },
     shouldDisplay() {
       return !this.isDismissed && this.diffFiles.length > 1;
     },
@@ -53,7 +43,7 @@ export default {
 </script>
 
 <template>
-  <div v-if="shouldDisplay" data-testid="root" :class="containerClasses" class="col-12">
+  <div v-if="shouldDisplay" data-testid="root" class="col-12">
     <gl-alert
       :dismissible="true"
       :title="__('Some changes are not shown')"

app/assets/javascripts/diffs/components/compare_versions.vue

@@ -3,7 +3,7 @@ import { GlTooltipDirective, GlIcon, GlLink, GlButtonGroup, GlButton, GlSprintf
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { __ } from '~/locale';
 import { setUrlParams } from '../../lib/utils/url_utility';
-import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants';
+import { EVT_EXPAND_ALL_FILES } from '../constants';
 import eventHub from '../event_hub';
 import CompareDropdownLayout from './compare_dropdown_layout.vue';
 import DiffStats from './diff_stats.vue';
@@ -24,11 +24,6 @@ export default {
     GlTooltip: GlTooltipDirective,
   },
   props: {
-    isLimitedContainer: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     diffFilesCountText: {
       type: String,
       required: false,
@@ -73,9 +68,6 @@ export default {
       return this.commit && (this.commit.next_commit_id || this.commit.prev_commit_id);
     },
   },
-  created() {
-    this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
-  },
   methods: {
     ...mapActions('diffs', ['setInlineDiffViewType', 'setParallelDiffViewType', 'setShowTreeList']),
     expandAllFiles() {
@@ -88,12 +80,7 @@ export default {
 
 <template>
   <div class="mr-version-controls border-top">
-    <div
-      class="mr-version-menus-container content-block"
-      :class="{
-        [CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer,
-      }"
-    >
+    <div class="mr-version-menus-container content-block">
       <gl-button
         v-if="hasChanges"
         v-gl-tooltip.hover

app/assets/javascripts/diffs/components/merge_conflict_warning.vue

 <script>
 import { GlButton, GlAlert, GlModalDirective } from '@gitlab/ui';
-import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants';
 
 export default {
   components: {
@@ -11,10 +10,6 @@ export default {
     GlModalDirective,
   },
   props: {
-    limited: {
-      type: Boolean,
-      required: true,
-    },
     mergeable: {
       type: Boolean,
       required: true,
@@ -24,18 +19,11 @@ export default {
       required: true,
     },
   },
-  computed: {
-    containerClasses() {
-      return {
-        [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited,
-      };
-    },
-  },
 };
 </script>
 
 <template>
-  <div :class="containerClasses">
+  <div>
     <gl-alert
       :dismissible="false"
       :title="__('There are merge conflicts')"

app/assets/javascripts/diffs/constants.js

@@ -50,9 +50,6 @@ export const NEW_LINE_KEY = 'new_line';
 export const TYPE_KEY = 'type';
 export const LEFT_LINE_KEY = 'left';
 
-export const CENTERED_LIMITED_CONTAINER_CLASSES =
-  'container-limited limit-container-width mx-lg-auto px-3';
-
 export const MAX_RENDERING_DIFF_LINES = 500;
 export const MAX_RENDERING_BULK_ROWS = 30;
 export const MIN_RENDERING_MS = 2;

app/controllers/projects/merge_requests_controller.rb

@@ -40,7 +40,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml)
     push_frontend_feature_flag(:diffs_virtual_scrolling, project, default_enabled: :yaml)
     push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml)
-    push_frontend_feature_flag(:mr_changes_fluid_layout, project, default_enabled: :yaml)
     push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml)
     push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:rebase_without_ci_ui, @project, default_enabled: :yaml)

config/feature_flags/development/mr_changes_fluid_layout.yml

----
-name: mr_changes_fluid_layout
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70815
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341809
-milestone: '14.4'
-type: development
-group: group::code review
-default_enabled: true

spec/frontend/diffs/components/app_spec.js

@@ -154,22 +154,6 @@ describe('diffs/components/app', () => {
     });
   });
 
-  it.each`
-    props                      | state                                                              | expected
-    ${{ isFluidLayout: true }} | ${{ isParallelView: false }}                                       | ${false}
-    ${{}}                      | ${{ isParallelView: false }}                                       | ${true}
-    ${{}}                      | ${{ showTreeList: true, diffFiles: [{}], isParallelView: false }}  | ${false}
-    ${{}}                      | ${{ showTreeList: false, diffFiles: [{}], isParallelView: false }} | ${true}
-    ${{}}                      | ${{ showTreeList: false, diffFiles: [], isParallelView: false }}   | ${true}
-  `(
-    'uses container-limiting classes ($expected) with state ($state) and props ($props)',
-    ({ props, state, expected }) => {
-      createComponent(props, ({ state: origState }) => Object.assign(origState.diffs, state));
-
-      expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(expected);
-    },
-  );
-
   it('displays loading icon on loading', () => {
     createComponent({}, ({ state }) => {
       state.diffs.isLoading = true;
@@ -498,7 +482,6 @@ describe('diffs/components/app', () => {
       expect(wrapper.find(CompareVersions).exists()).toBe(true);
       expect(wrapper.find(CompareVersions).props()).toEqual(
         expect.objectContaining({
-          isLimitedContainer: false,
           diffFilesCountText: null,
         }),
       );

spec/frontend/diffs/components/collapsed_files_warning_spec.js

@@ -2,7 +2,7 @@ import { shallowMount, mount } from '@vue/test-utils';
 import Vue, { nextTick } from 'vue';
 import Vuex from 'vuex';
 import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
-import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '~/diffs/constants';
+import { EVT_EXPAND_ALL_FILES } from '~/diffs/constants';
 import eventHub from '~/diffs/event_hub';
 import createStore from '~/diffs/store/modules';
 
@@ -13,7 +13,6 @@ const propsData = {
   mergeable: true,
   resolutionPath: 'a-path',
 };
-const limitedClasses = CENTERED_LIMITED_CONTAINER_CLASSES.split(' ');
 
 async function files(store, count) {
   const copies = Array(count).fill(file);
@@ -51,20 +50,6 @@ describe('CollapsedFilesWarning', () => {
   });
 
   describe('when there is more than one file', () => {
-    it.each`
-      limited  | containerClasses
-      ${true}  | ${limitedClasses}
-      ${false} | ${[]}
-    `(
-      'has the correct container classes when limited is $limited',
-      async ({ limited, containerClasses }) => {
-        createComponent({ limited });
-        await files(store, 2);
-
-        expect(wrapper.classes()).toEqual(['col-12'].concat(containerClasses));
-      },
-    );
-
     it.each`
       present  | dismissed
       ${false} | ${true}

spec/frontend/diffs/components/compare_versions_spec.js

@@ -38,7 +38,6 @@ describe('CompareVersions', () => {
       },
     });
   };
-  const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width');
   const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown');
   const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown');
   const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons');
@@ -98,18 +97,6 @@ describe('CompareVersions', () => {
       expect(inlineBtn.html()).toContain('Inline');
       expect(parallelBtn.html()).toContain('Side-by-side');
     });
-
-    it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
-      createWrapper({ isLimitedContainer: true });
-
-      expect(findLimitedContainer().exists()).toBe(true);
-    });
-
-    it('does not add container-limiting classes when showFileTree is false with inline diffs', () => {
-      createWrapper({ isLimitedContainer: false });
-
-      expect(findLimitedContainer().exists()).toBe(false);
-    });
   });
 
   describe('noChangedFiles', () => {

spec/frontend/diffs/components/merge_conflict_warning_spec.js

 import { shallowMount, mount } from '@vue/test-utils';
 import MergeConflictWarning from '~/diffs/components/merge_conflict_warning.vue';
-import { CENTERED_LIMITED_CONTAINER_CLASSES } from '~/diffs/constants';
 
 const propsData = {
   limited: true,
   mergeable: true,
   resolutionPath: 'a-path',
 };
-const limitedClasses = CENTERED_LIMITED_CONTAINER_CLASSES.split(' ');
 
 function findResolveButton(wrapper) {
   return wrapper.find('.gl-alert-actions a.gl-button:first-child');
@@ -31,19 +29,6 @@ describe('MergeConflictWarning', () => {
     wrapper.destroy();
   });
 
-  it.each`
-    limited  | containerClasses
-    ${true}  | ${limitedClasses}
-    ${false} | ${[]}
-  `(
-    'has the correct container classes when limited is $limited',
-    ({ limited, containerClasses }) => {
-      createComponent({ limited });
-
-      expect(wrapper.classes()).toEqual(containerClasses);
-    },
-  );
-
   it.each`
     present  | resolutionPath
     ${false} | ${''}