Commit 8f09a76f authored by Thomas Randolph's avatar Thomas Randolph Committed by Phil Hughes

Expand diff_file collapsed UIs to be significantly more obvious

- Expand the collapsed file body vertically
- Color the collapsed file body orange
- Expand the only-blob-available body vertically
- Color the only-blob-available body orange
- Convert the control to expand collapsed files into a button
parent 2e9af7dc
<script> <script>
import { mapActions, mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { sprintf } from '~/locale'; import { sprintf } from '~/locale';
import { deprecatedCreateFlash as createFlash } from '~/flash'; import { deprecatedCreateFlash as createFlash } from '~/flash';
...@@ -19,6 +19,7 @@ export default { ...@@ -19,6 +19,7 @@ export default {
components: { components: {
DiffFileHeader, DiffFileHeader,
DiffContent, DiffContent,
GlButton,
GlLoadingIcon, GlLoadingIcon,
}, },
directives: { directives: {
...@@ -267,16 +268,21 @@ export default { ...@@ -267,16 +268,21 @@ export default {
<div v-safe-html="errorMessage" class="nothing-here-block"></div> <div v-safe-html="errorMessage" class="nothing-here-block"></div>
</div> </div>
<template v-else> <template v-else>
<div v-show="showWarning" class="nothing-here-block diff-collapsed"> <div
{{ $options.i18n.collapsed }} v-show="showWarning"
<a class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base"
class="click-to-expand" >
data-testid="toggle-link" <p class="gl-mb-8">
href="#" {{ $options.i18n.autoCollapsed }}
</p>
<gl-button
data-testid="expand-button"
category="secondary"
variant="warning"
@click.prevent="handleToggle" @click.prevent="handleToggle"
> >
{{ $options.i18n.expand }} {{ $options.i18n.expand }}
</a> </gl-button>
</div> </div>
<diff-content <diff-content
v-show="showContent" v-show="showContent"
......
...@@ -13,6 +13,6 @@ export const DIFF_FILE = { ...@@ -13,6 +13,6 @@ export const DIFF_FILE = {
), ),
fork: __('Fork'), fork: __('Fork'),
cancel: __('Cancel'), cancel: __('Cancel'),
collapsed: __('This diff is collapsed.'), autoCollapsed: __('Files with large changes are collapsed by default.'),
expand: __('Click to expand it.'), expand: __('Expand file'),
}; };
---
title: Expand Diff File collapsed UI to be significantly more obvious
merge_request: 46286
author:
type: changed
...@@ -10857,6 +10857,9 @@ msgstr "" ...@@ -10857,6 +10857,9 @@ msgstr ""
msgid "Expand approvers" msgid "Expand approvers"
msgstr "" msgstr ""
msgid "Expand file"
msgstr ""
msgid "Expand milestones" msgid "Expand milestones"
msgstr "" msgstr ""
...@@ -11558,6 +11561,9 @@ msgstr "" ...@@ -11558,6 +11561,9 @@ msgstr ""
msgid "Files breadcrumb" msgid "Files breadcrumb"
msgstr "" msgstr ""
msgid "Files with large changes are collapsed by default."
msgstr ""
msgid "Files, directories, and submodules in the path %{path} for commit reference %{ref}" msgid "Files, directories, and submodules in the path %{path} for commit reference %{ref}"
msgstr "" msgstr ""
......
...@@ -15,11 +15,11 @@ RSpec.describe 'User expands diff', :js do ...@@ -15,11 +15,11 @@ RSpec.describe 'User expands diff', :js do
it 'allows user to expand diff' do it 'allows user to expand diff' do
page.within find('[id="19763941ab80e8c09871c0a425f0560d9053bcb3"]') do page.within find('[id="19763941ab80e8c09871c0a425f0560d9053bcb3"]') do
click_link 'Click to expand it.' find('[data-testid="expand-button"]').click
wait_for_requests wait_for_requests
expect(page).not_to have_content('Click to expand it.') expect(page).not_to have_content('Expand file')
expect(page).to have_selector('.code') expect(page).to have_selector('.code')
end end
end end
......
...@@ -90,7 +90,7 @@ function createComponent({ file }) { ...@@ -90,7 +90,7 @@ function createComponent({ file }) {
const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent); const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent);
const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]'); const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]');
const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]'); const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]');
const findToggleLinks = wrapper => wrapper.findAll('[data-testid="toggle-link"]'); const findToggleButton = wrapper => wrapper.find('[data-testid="expand-button"]');
const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile'); const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile');
const isDisplayNone = element => element.style.display === 'none'; const isDisplayNone = element => element.style.display === 'none';
...@@ -187,9 +187,11 @@ describe('DiffFile', () => { ...@@ -187,9 +187,11 @@ describe('DiffFile', () => {
makeFileAutomaticallyCollapsed(store); makeFileAutomaticallyCollapsed(store);
}); });
it('should show the collapsed file warning with expansion link', () => { it('should show the collapsed file warning with expansion button', () => {
expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed'); expect(findDiffContentArea(wrapper).html()).toContain(
expect(findToggleLinks(wrapper).length).toEqual(1); 'Files with large changes are collapsed by default.',
);
expect(findToggleButton(wrapper).exists()).toBe(true);
}); });
it('should style the component so that it `.has-body` for layout purposes', () => { it('should style the component so that it `.has-body` for layout purposes', () => {
...@@ -292,8 +294,10 @@ describe('DiffFile', () => { ...@@ -292,8 +294,10 @@ describe('DiffFile', () => {
await wrapper.vm.$nextTick(); await wrapper.vm.$nextTick();
expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed'); expect(findDiffContentArea(wrapper).html()).toContain(
expect(findToggleLinks(wrapper).length).toEqual(1); 'Files with large changes are collapsed by default.',
);
expect(findToggleButton(wrapper).exists()).toBe(true);
}); });
it.each` it.each`
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment