Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
e54d7ee3
Commit
e54d7ee3
authored
Apr 05, 2019
by
GitLab Bot
Browse files
Options
Browse Files
Download
Plain Diff
Automatic merge of gitlab-org/gitlab-ce master
parents
4deb523c
e3fedd68
Changes
14
Hide whitespace changes
Inline
Side-by-side
Showing
14 changed files
with
179 additions
and
87 deletions
+179
-87
app/assets/javascripts/breakpoints.js
app/assets/javascripts/breakpoints.js
+3
-0
app/assets/javascripts/diffs/components/app.vue
app/assets/javascripts/diffs/components/app.vue
+12
-3
app/assets/javascripts/diffs/components/compare_versions.vue
app/assets/javascripts/diffs/components/compare_versions.vue
+16
-2
app/assets/javascripts/diffs/components/diff_file_header.vue
app/assets/javascripts/diffs/components/diff_file_header.vue
+74
-63
app/assets/javascripts/diffs/constants.js
app/assets/javascripts/diffs/constants.js
+3
-0
app/assets/javascripts/lib/utils/common_utils.js
app/assets/javascripts/lib/utils/common_utils.js
+4
-5
app/assets/stylesheets/framework/files.scss
app/assets/stylesheets/framework/files.scss
+4
-0
app/assets/stylesheets/pages/diff.scss
app/assets/stylesheets/pages/diff.scss
+6
-2
app/assets/stylesheets/pages/merge_requests.scss
app/assets/stylesheets/pages/merge_requests.scss
+9
-7
changelogs/unreleased/57364-improve-diff-nav-header.yml
changelogs/unreleased/57364-improve-diff-nav-header.yml
+5
-0
spec/javascripts/diffs/components/app_spec.js
spec/javascripts/diffs/components/app_spec.js
+18
-0
spec/javascripts/diffs/components/compare_versions_spec.js
spec/javascripts/diffs/components/compare_versions_spec.js
+20
-0
spec/javascripts/diffs/components/diff_file_header_spec.js
spec/javascripts/diffs/components/diff_file_header_spec.js
+2
-2
spec/javascripts/lib/utils/common_utils_spec.js
spec/javascripts/lib/utils/common_utils_spec.js
+3
-3
No files found.
app/assets/javascripts/breakpoints.js
View file @
e54d7ee3
...
...
@@ -14,6 +14,9 @@ const BreakpointInstance = {
return
breakpoint
;
},
isDesktop
()
{
return
[
'
lg
'
,
'
md
'
].
includes
(
this
.
getBreakpointSize
);
},
};
export
default
BreakpointInstance
;
app/assets/javascripts/diffs/components/app.vue
View file @
e54d7ee3
...
...
@@ -20,6 +20,7 @@ import {
MAX_TREE_WIDTH
,
TREE_HIDE_STATS_WIDTH
,
MR_TREE_SHOW_KEY
,
CENTERED_LIMITED_CONTAINER_CLASSES
,
}
from
'
../constants
'
;
export
default
{
...
...
@@ -114,6 +115,9 @@ export default {
hideFileStats
()
{
return
this
.
treeWidth
<=
TREE_HIDE_STATS_WIDTH
;
},
isLimitedContainer
()
{
return
!
this
.
showTreeList
&&
!
this
.
isParallelView
;
},
},
watch
:
{
diffViewType
()
{
...
...
@@ -148,6 +152,7 @@ export default {
this
.
adjustView
();
eventHub
.
$once
(
'
fetchedNotesData
'
,
this
.
setDiscussions
);
eventHub
.
$once
(
'
fetchDiffData
'
,
this
.
fetchData
);
this
.
CENTERED_LIMITED_CONTAINER_CLASSES
=
CENTERED_LIMITED_CONTAINER_CLASSES
;
},
beforeDestroy
()
{
eventHub
.
$off
(
'
fetchDiffData
'
,
this
.
fetchData
);
...
...
@@ -202,8 +207,6 @@ export default {
adjustView
()
{
if
(
this
.
shouldShow
)
{
this
.
$nextTick
(()
=>
{
window
.
mrTabs
.
resetViewContainer
();
window
.
mrTabs
.
expandViewContainer
(
this
.
showTreeList
);
this
.
setEventListeners
();
});
}
else
{
...
...
@@ -256,6 +259,7 @@ export default {
:merge-request-diffs=
"mergeRequestDiffs"
:merge-request-diff=
"mergeRequestDiff"
:target-branch=
"targetBranch"
:is-limited-container=
"isLimitedContainer"
/>
<hidden-files-warning
...
...
@@ -285,7 +289,12 @@ export default {
/>
<tree-list
:hide-file-stats=
"hideFileStats"
/>
</div>
<div
class=
"diff-files-holder"
>
<div
class=
"diff-files-holder"
:class=
"
{
[CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer,
}"
>
<commit-widget
v-if=
"commit"
:commit=
"commit"
/>
<template
v-if=
"renderDiffFiles"
>
<diff-file
...
...
app/assets/javascripts/diffs/components/compare_versions.vue
View file @
e54d7ee3
...
...
@@ -7,6 +7,7 @@ import Icon from '~/vue_shared/components/icon.vue';
import
CompareVersionsDropdown
from
'
./compare_versions_dropdown.vue
'
;
import
SettingsDropdown
from
'
./settings_dropdown.vue
'
;
import
DiffStats
from
'
./diff_stats.vue
'
;
import
{
CENTERED_LIMITED_CONTAINER_CLASSES
}
from
'
../constants
'
;
export
default
{
components
:
{
...
...
@@ -35,6 +36,11 @@ export default {
required
:
false
,
default
:
null
,
},
isLimitedContainer
:
{
type
:
Boolean
,
required
:
false
,
default
:
false
,
},
},
computed
:
{
...
mapGetters
(
'
diffs
'
,
[
'
hasCollapsedFile
'
,
'
diffFilesLength
'
]),
...
...
@@ -62,6 +68,9 @@ export default {
return
this
.
mergeRequestDiff
.
base_version_path
;
},
},
created
()
{
this
.
CENTERED_LIMITED_CONTAINER_CLASSES
=
CENTERED_LIMITED_CONTAINER_CLASSES
;
},
mounted
()
{
polyfillSticky
(
this
.
$el
);
},
...
...
@@ -77,8 +86,13 @@ export default {
</
script
>
<
template
>
<div
class=
"mr-version-controls"
:class=
"
{ 'is-fileTreeOpen': showTreeList }">
<div
class=
"mr-version-menus-container content-block"
>
<div
class=
"mr-version-controls border-top border-bottom"
>
<div
class=
"mr-version-menus-container content-block"
:class=
"
{
[CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer,
}"
>
<button
v-gl-tooltip
.
hover
type=
"button"
...
...
app/assets/javascripts/diffs/components/diff_file_header.vue
View file @
e54d7ee3
<
script
>
import
_
from
'
underscore
'
;
import
{
mapActions
,
mapGetters
}
from
'
vuex
'
;
import
{
polyfillSticky
}
from
'
~/lib/utils/sticky
'
;
import
{
polyfillSticky
,
stickyMonitor
}
from
'
~/lib/utils/sticky
'
;
import
ClipboardButton
from
'
~/vue_shared/components/clipboard_button.vue
'
;
import
Icon
from
'
~/vue_shared/components/icon.vue
'
;
import
FileIcon
from
'
~/vue_shared/components/file_icon.vue
'
;
...
...
@@ -11,7 +11,7 @@ import { __, s__, sprintf } from '~/locale';
import
{
diffViewerModes
}
from
'
~/ide/constants
'
;
import
EditButton
from
'
./edit_button.vue
'
;
import
DiffStats
from
'
./diff_stats.vue
'
;
import
{
scrollToElement
}
from
'
~/lib/utils/common_utils
'
;
import
{
scrollToElement
,
contentTop
}
from
'
~/lib/utils/common_utils
'
;
export
default
{
components
:
{
...
...
@@ -137,9 +137,20 @@ export default {
isModeChanged
()
{
return
this
.
diffFile
.
viewer
.
name
===
diffViewerModes
.
mode_changed
;
},
showExpandDiffToFullFileEnabled
()
{
return
gon
.
features
.
expandDiffFullFile
&&
!
this
.
diffFile
.
is_fully_expanded
;
},
expandDiffToFullFileTitle
()
{
if
(
this
.
diffFile
.
isShowingFullFile
)
{
return
s__
(
'
MRDiff|Show changes only
'
);
}
return
s__
(
'
MRDiff|Show full file
'
);
},
},
mounted
()
{
polyfillSticky
(
this
.
$refs
.
header
);
const
fileHeaderHeight
=
this
.
$refs
.
header
.
clientHeight
;
stickyMonitor
(
this
.
$refs
.
header
,
contentTop
()
-
fileHeaderHeight
-
1
,
false
);
},
methods
:
{
...
mapActions
(
'
diffs
'
,
[
'
toggleFileDiscussions
'
,
'
toggleFullDiff
'
]),
...
...
@@ -243,70 +254,70 @@ export default {
class=
"file-actions d-none d-sm-block"
>
<diff-stats
:added-lines=
"diffFile.added_lines"
:removed-lines=
"diffFile.removed_lines"
/>
<template
v-if=
"diffFile.blob && diffFile.blob.readable_text"
>
<button
:disabled=
"!diffHasDiscussions(diffFile)"
:class=
"
{ active: hasExpandedDiscussions }"
:title="s__('MergeRequests|Toggle comments for this file')"
class="js-btn-vue-toggle-comments btn"
type="button"
@click="handleToggleDiscussions"
>
<icon
name=
"comment"
/>
</button>
<edit-button
v-if=
"!diffFile.deleted_file"
:can-current-user-fork=
"canCurrentUserFork"
:edit-path=
"diffFile.edit_path"
:can-modify-blob=
"diffFile.can_modify_blob"
@
showForkMessage=
"showForkMessage"
/>
</
template
>
<div
class=
"btn-group"
role=
"group"
>
<template
v-if=
"diffFile.blob && diffFile.blob.readable_text"
>
<button
:disabled=
"!diffHasDiscussions(diffFile)"
:class=
"
{ active: hasExpandedDiscussions }"
:title="s__('MergeRequests|Toggle comments for this file')"
class="js-btn-vue-toggle-comments btn"
type="button"
@click="handleToggleDiscussions"
>
<icon
name=
"comment"
/>
</button>
<a
v-if=
"diffFile.replaced_view_path"
:href=
"diffFile.replaced_view_path"
class=
"btn view-file js-view-replaced-file"
v-html=
"viewReplacedFileButtonText"
>
</a>
<gl-tooltip
:target=
"() => $refs.viewButton"
placement=
"bottom"
>
<span
v-html=
"viewFileButtonText"
></span>
</gl-tooltip>
<gl-button
ref=
"viewButton"
:href=
"diffFile.view_path"
target=
"blank"
class=
"view-file js-view-file-button"
>
<icon
name=
"external-link"
/>
</gl-button>
<gl-button
v-if=
"!diffFile.is_fully_expanded"
class=
"expand-file js-expand-file"
@
click=
"toggleFullDiff(diffFile.file_path)"
>
<
template
v-if=
"diffFile.isShowingFullFile"
>
{{
s__
(
'
MRDiff|Show changes only
'
)
}}
</
template
>
<
template
v-else
>
{{
s__
(
'
MRDiff|Show full file
'
)
}}
<edit-button
v-if=
"!diffFile.deleted_file"
:can-current-user-fork=
"canCurrentUserFork"
:edit-path=
"diffFile.edit_path"
:can-modify-blob=
"diffFile.can_modify_blob"
@
showForkMessage=
"showForkMessage"
/>
</
template
>
<gl-loading-icon
v-if=
"diffFile.isLoadingFullFile"
inline
/>
</gl-button>
<a
v-if=
"diffFile.external_url"
v-gl-tooltip
.
hover
:href=
"diffFile.external_url"
:title=
"`View on ${diffFile.formatted_external_url}`"
target=
"_blank"
rel=
"noopener noreferrer"
class=
"btn btn-file-option js-external-url"
>
<icon
name=
"external-link"
/>
</a>
<a
v-if=
"diffFile.replaced_view_path"
:href=
"diffFile.replaced_view_path"
class=
"btn view-file js-view-replaced-file"
v-html=
"viewReplacedFileButtonText"
>
</a>
<gl-button
v-if=
"!diffFile.is_fully_expanded"
ref=
"expandDiffToFullFileButton"
v-gl-tooltip
.
hover
:title=
"expandDiffToFullFileTitle"
class=
"expand-file js-expand-file"
@
click=
"toggleFullDiff(diffFile.file_path)"
>
<gl-loading-icon
v-if=
"diffFile.isLoadingFullFile"
color=
"dark"
inline
/>
<icon
v-else-if=
"diffFile.isShowingFullFile"
name=
"doc-changes"
/>
<icon
v-else
name=
"doc-expand"
/>
</gl-button>
<gl-button
ref=
"viewButton"
v-gl-tooltip
.
hover
:href=
"diffFile.view_path"
target=
"blank"
class=
"view-file js-view-file-button"
:title=
"viewFileButtonText"
>
<icon
name=
"external-link"
/>
</gl-button>
<a
v-if=
"diffFile.external_url"
v-gl-tooltip
.
hover
:href=
"diffFile.external_url"
:title=
"`View on ${diffFile.formatted_external_url}`"
target=
"_blank"
rel=
"noopener noreferrer"
class=
"btn btn-file-option js-external-url"
>
<icon
name=
"external-link"
/>
</a>
</div>
</div>
</div>
</template>
app/assets/javascripts/diffs/constants.js
View file @
e54d7ee3
...
...
@@ -47,3 +47,6 @@ export const OLD_LINE_KEY = 'old_line';
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-auto px-3
'
;
app/assets/javascripts/lib/utils/common_utils.js
View file @
e54d7ee3
...
...
@@ -7,7 +7,7 @@ import axios from './axios_utils';
import
{
getLocationHash
}
from
'
./url_utility
'
;
import
{
convertToCamelCase
}
from
'
./text_utility
'
;
import
{
isObject
}
from
'
./type_utility
'
;
import
B
reakpointInstance
from
'
../../breakpoints
'
;
import
b
reakpointInstance
from
'
../../breakpoints
'
;
export
const
getPagePath
=
(
index
=
0
)
=>
{
const
page
=
$
(
'
body
'
).
attr
(
'
data-page
'
)
||
''
;
...
...
@@ -198,11 +198,10 @@ export const contentTop = () => {
const
mrTabsHeight
=
$
(
'
.merge-request-tabs
'
).
outerHeight
()
||
0
;
const
headerHeight
=
$
(
'
.navbar-gitlab
'
).
outerHeight
()
||
0
;
const
diffFilesChanged
=
$
(
'
.js-diff-files-changed
'
).
outerHeight
()
||
0
;
const
mdScreenOrBigger
=
[
'
lg
'
,
'
md
'
].
includes
(
BreakpointInstance
.
getBreakpointSize
()
);
const
isDesktop
=
breakpointInstance
.
isDesktop
(
);
const
diffFileTitleBar
=
(
mdScreenOrBigger
&&
$
(
'
.diff-file .file-title-flex-parent:visible
'
).
outerHeight
())
||
0
;
const
compareVersionsHeaderHeight
=
(
mdScreenOrBigger
&&
$
(
'
.mr-version-controls
'
).
outerHeight
())
||
0
;
(
isDesktop
&&
$
(
'
.diff-file .file-title-flex-parent:visible
'
).
outerHeight
())
||
0
;
const
compareVersionsHeaderHeight
=
(
isDesktop
&&
$
(
'
.mr-version-controls
'
).
outerHeight
())
||
0
;
return
(
perfBar
+
...
...
app/assets/stylesheets/framework/files.scss
View file @
e54d7ee3
...
...
@@ -331,6 +331,10 @@ span.idiff {
padding
:
5px
$gl-padding
;
margin
:
0
;
border-radius
:
$border-radius-default
$border-radius-default
0
0
;
&
.is-stuck
{
border-radius
:
0
;
}
}
.file-header-content
{
...
...
app/assets/stylesheets/pages/diff.scss
View file @
e54d7ee3
...
...
@@ -7,12 +7,15 @@
cursor
:
pointer
;
@media
(
min-width
:
map-get
(
$grid-breakpoints
,
md
))
{
$mr-file-header-top
:
$mr-version-controls-height
+
$header-height
+
$mr-tabs-height
;
// The `-1` below is to prevent two borders from clashing up against eachother -
// the bottom of the compare-versions header and the top of the file header
$mr-file-header-top
:
$mr-version-controls-height
+
$header-height
+
$mr-tabs-height
-
1
;
position
:
-
webkit-sticky
;
position
:
sticky
;
top
:
$mr-file-header-top
;
z-index
:
102
;
height
:
$mr-version-controls-height
;
&
:
:
before
{
content
:
''
;
...
...
@@ -54,7 +57,8 @@
background-color
:
$gray-normal
;
}
a
{
a
,
button
{
color
:
$gray-700
;
}
...
...
app/assets/stylesheets/pages/merge_requests.scss
View file @
e54d7ee3
...
...
@@ -736,7 +736,6 @@
background
:
$gray-light
;
color
:
$gl-text-color
;
margin-top
:
-1px
;
border-top
:
1px
solid
$border-color
;
.mr-version-menus-container
{
display
:
flex
;
...
...
@@ -759,6 +758,7 @@
.content-block
{
padding
:
$gl-padding-top
$gl-padding
;
border-bottom
:
0
;
}
.comments-disabled-notif
{
...
...
@@ -783,16 +783,18 @@
padding-right
:
5px
;
}
// Shortening button height by 1px to make compare-versions
// header 56px and fit into our 8px design grid
button
{
height
:
34px
;
}
@include
media-breakpoint-up
(
md
)
{
position
:
-
webkit-sticky
;
position
:
sticky
;
top
:
$header-height
+
$mr-tabs-height
;
width
:
100%
;
&
.is-fileTreeOpen
{
margin-left
:
-16px
;
width
:
calc
(
100%
+
32px
);
}
margin-left
:
-16px
;
width
:
calc
(
100%
+
32px
);
.mr-version-menus-container
{
flex-wrap
:
nowrap
;
...
...
changelogs/unreleased/57364-improve-diff-nav-header.yml
0 → 100644
View file @
e54d7ee3
---
title
:
Make stylistic improvements to diff nav header
merge_request
:
26557
author
:
type
:
fixed
spec/javascripts/diffs/components/app_spec.js
View file @
e54d7ee3
...
...
@@ -57,6 +57,24 @@ describe('diffs/components/app', () => {
wrapper
.
destroy
();
});
it
(
'
adds container-limiting classes when showFileTree is false with inline diffs
'
,
()
=>
{
createComponent
({},
({
state
})
=>
{
state
.
diffs
.
showTreeList
=
false
;
state
.
diffs
.
isParallelView
=
false
;
});
expect
(
wrapper
.
contains
(
'
.container-limited.limit-container-width
'
)).
toBe
(
true
);
});
it
(
'
does not add container-limiting classes when showFileTree is false with inline diffs
'
,
()
=>
{
createComponent
({},
({
state
})
=>
{
state
.
diffs
.
showTreeList
=
true
;
state
.
diffs
.
isParallelView
=
false
;
});
expect
(
wrapper
.
contains
(
'
.container-limited.limit-container-width
'
)).
toBe
(
false
);
});
it
(
'
displays loading icon on loading
'
,
()
=>
{
createComponent
({},
({
state
})
=>
{
state
.
diffs
.
isLoading
=
true
;
...
...
spec/javascripts/diffs/components/compare_versions_spec.js
View file @
e54d7ee3
...
...
@@ -66,6 +66,26 @@ describe('CompareVersions', () => {
expect
(
inlineBtn
.
innerHTML
).
toContain
(
'
Inline
'
);
expect
(
parallelBtn
.
innerHTML
).
toContain
(
'
Side-by-side
'
);
});
it
(
'
adds container-limiting classes when showFileTree is false with inline diffs
'
,
()
=>
{
vm
.
isLimitedContainer
=
true
;
vm
.
$nextTick
(()
=>
{
const
limitedContainer
=
vm
.
$el
.
querySelector
(
'
.container-limited.limit-container-width
'
);
expect
(
limitedContainer
).
not
.
toBeNull
();
});
});
it
(
'
does not add container-limiting classes when showFileTree is false with inline diffs
'
,
()
=>
{
vm
.
isLimitedContainer
=
false
;
vm
.
$nextTick
(()
=>
{
const
limitedContainer
=
vm
.
$el
.
querySelector
(
'
.container-limited.limit-container-width
'
);
expect
(
limitedContainer
).
toBeNull
();
});
});
});
describe
(
'
setInlineDiffViewType
'
,
()
=>
{
...
...
spec/javascripts/diffs/components/diff_file_header_spec.js
View file @
e54d7ee3
...
...
@@ -672,7 +672,7 @@ describe('diff_file_header', () => {
vm
=
mountComponentWithStore
(
Component
,
{
props
,
store
});
expect
(
vm
.
$el
.
querySelector
(
'
.
js-expand-file
'
).
textContent
).
toContain
(
'
Show changes only
'
);
expect
(
vm
.
$el
.
querySelector
(
'
.
ic-doc-changes
'
)).
not
.
toBeNull
(
);
});
it
(
'
shows expand text
'
,
()
=>
{
...
...
@@ -680,7 +680,7 @@ describe('diff_file_header', () => {
vm
=
mountComponentWithStore
(
Component
,
{
props
,
store
});
expect
(
vm
.
$el
.
querySelector
(
'
.
js-expand-file
'
).
textContent
).
toContain
(
'
Show full file
'
);
expect
(
vm
.
$el
.
querySelector
(
'
.
ic-doc-expand
'
)).
not
.
toBeNull
(
);
});
it
(
'
renders loading icon
'
,
()
=>
{
...
...
spec/javascripts/lib/utils/common_utils_spec.js
View file @
e54d7ee3
...
...
@@ -2,7 +2,7 @@ import axios from '~/lib/utils/axios_utils';
import
*
as
commonUtils
from
'
~/lib/utils/common_utils
'
;
import
MockAdapter
from
'
axios-mock-adapter
'
;
import
{
faviconDataUrl
,
overlayDataUrl
,
faviconWithOverlayDataUrl
}
from
'
./mock_data
'
;
import
B
reakpointInstance
from
'
~/breakpoints
'
;
import
b
reakpointInstance
from
'
~/breakpoints
'
;
const
PIXEL_TOLERANCE
=
0.2
;
...
...
@@ -383,7 +383,7 @@ describe('common_utils', () => {
describe
(
'
contentTop
'
,
()
=>
{
it
(
'
does not add height for fileTitle or compareVersionsHeader if screen is too small
'
,
()
=>
{
spyOn
(
BreakpointInstance
,
'
getBreakpointSize
'
).
and
.
returnValue
(
'
sm
'
);
spyOn
(
breakpointInstance
,
'
isDesktop
'
).
and
.
returnValue
(
false
);
setFixtures
(
`
<div class="diff-file file-title-flex-parent">
...
...
@@ -398,7 +398,7 @@ describe('common_utils', () => {
});
it
(
'
adds height for fileTitle and compareVersionsHeader screen is large enough
'
,
()
=>
{
spyOn
(
BreakpointInstance
,
'
getBreakpointSize
'
).
and
.
returnValue
(
'
lg
'
);
spyOn
(
breakpointInstance
,
'
isDesktop
'
).
and
.
returnValue
(
true
);
setFixtures
(
`
<div class="diff-file file-title-flex-parent">
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment