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
0
Merge Requests
0
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
Léo-Paul Géneau
gitlab-ce
Commits
fb44cb3d
Commit
fb44cb3d
authored
6 years ago
by
Tim Zallmann
Committed by
Fatih Acet
6 years ago
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Performance Improvements to Vue MR page
parent
3a3233a5
Changes
9
Show whitespace changes
Inline
Side-by-side
Showing
9 changed files
with
141 additions
and
93 deletions
+141
-93
app/assets/javascripts/diffs/components/app.vue
app/assets/javascripts/diffs/components/app.vue
+1
-1
app/assets/javascripts/merge_request_tabs.js
app/assets/javascripts/merge_request_tabs.js
+92
-61
app/assets/javascripts/notes/components/notes_app.vue
app/assets/javascripts/notes/components/notes_app.vue
+1
-1
app/helpers/merge_requests_helper.rb
app/helpers/merge_requests_helper.rb
+1
-1
app/views/projects/merge_requests/creations/_new_submit.html.haml
...s/projects/merge_requests/creations/_new_submit.html.haml
+21
-16
spec/helpers/merge_requests_helper_spec.rb
spec/helpers/merge_requests_helper_spec.rb
+1
-1
spec/javascripts/helpers/init_vue_mr_page_helper.js
spec/javascripts/helpers/init_vue_mr_page_helper.js
+8
-4
spec/javascripts/merge_request_spec.js
spec/javascripts/merge_request_spec.js
+15
-8
spec/javascripts/merge_request_tabs_spec.js
spec/javascripts/merge_request_tabs_spec.js
+1
-0
No files found.
app/assets/javascripts/diffs/components/app.vue
View file @
fb44cb3d
...
...
@@ -145,7 +145,7 @@ export default {
</
script
>
<
template
>
<div
v-
if
=
"shouldShow"
>
<div
v-
show
=
"shouldShow"
>
<div
v-if=
"isLoading"
class=
"loading"
...
...
This diff is collapsed.
Click to expand it.
app/assets/javascripts/merge_request_tabs.js
View file @
fb44cb3d
...
...
@@ -69,12 +69,23 @@ let { location } = window;
export
default
class
MergeRequestTabs
{
constructor
({
action
,
setUrl
,
stubLocation
}
=
{})
{
const
mergeRequestTabs
=
document
.
querySelector
(
'
.js-tabs-affix
'
);
this
.
mergeRequestTabs
=
document
.
querySelector
(
'
.merge-request-tabs-container
'
);
this
.
mergeRequestTabsAll
=
this
.
mergeRequestTabs
&&
this
.
mergeRequestTabs
.
querySelectorAll
?
this
.
mergeRequestTabs
.
querySelectorAll
(
'
.merge-request-tabs li
'
)
:
null
;
this
.
mergeRequestTabPanes
=
document
.
querySelector
(
'
#diff-notes-app
'
);
this
.
mergeRequestTabPanesAll
=
this
.
mergeRequestTabPanes
&&
this
.
mergeRequestTabPanes
.
querySelectorAll
?
this
.
mergeRequestTabPanes
.
querySelectorAll
(
'
.tab-pane
'
)
:
null
;
const
navbar
=
document
.
querySelector
(
'
.navbar-gitlab
'
);
const
peek
=
document
.
getElementById
(
'
js-peek
'
);
const
paddingTop
=
16
;
this
.
commitsTab
=
document
.
querySelector
(
'
.tab-content .commits.tab-pane
'
);
this
.
currentTab
=
null
;
this
.
diffsLoaded
=
false
;
this
.
pipelinesLoaded
=
false
;
this
.
commitsLoaded
=
false
;
...
...
@@ -84,15 +95,15 @@ export default class MergeRequestTabs {
this
.
setUrl
=
setUrl
!==
undefined
?
setUrl
:
true
;
this
.
setCurrentAction
=
this
.
setCurrentAction
.
bind
(
this
);
this
.
tabShown
=
this
.
tabShown
.
bind
(
this
);
this
.
showTab
=
this
.
show
Tab
.
bind
(
this
);
this
.
clickTab
=
this
.
click
Tab
.
bind
(
this
);
this
.
stickyTop
=
navbar
?
navbar
.
offsetHeight
-
paddingTop
:
0
;
if
(
peek
)
{
this
.
stickyTop
+=
peek
.
offsetHeight
;
}
if
(
mergeRequestTabs
)
{
this
.
stickyTop
+=
mergeRequestTabs
.
offsetHeight
;
if
(
this
.
mergeRequestTabs
)
{
this
.
stickyTop
+=
this
.
mergeRequestTabs
.
offsetHeight
;
}
if
(
stubLocation
)
{
...
...
@@ -100,25 +111,22 @@ export default class MergeRequestTabs {
}
this
.
bindEvents
();
this
.
activateTab
(
action
);
if
(
this
.
mergeRequestTabs
&&
this
.
mergeRequestTabs
.
querySelector
(
`a[data-action='
${
action
}
']`
)
&&
this
.
mergeRequestTabs
.
querySelector
(
`a[data-action='
${
action
}
']`
).
click
)
this
.
mergeRequestTabs
.
querySelector
(
`a[data-action='
${
action
}
']`
).
click
();
this
.
initAffix
();
}
bindEvents
()
{
$
(
document
)
.
on
(
'
shown.bs.tab
'
,
'
.merge-request-tabs a[data-toggle="tab"]
'
,
this
.
tabShown
)
.
on
(
'
click
'
,
'
.js-show-tab
'
,
this
.
showTab
);
$
(
'
.merge-request-tabs a[data-toggle="tab"]
'
).
on
(
'
click
'
,
this
.
clickTab
);
$
(
'
.merge-request-tabs a[data-toggle="tabvue"]
'
).
on
(
'
click
'
,
this
.
clickTab
);
}
// Used in tests
unbindEvents
()
{
$
(
document
)
.
off
(
'
shown.bs.tab
'
,
'
.merge-request-tabs a[data-toggle="tab"]
'
,
this
.
tabShown
)
.
off
(
'
click
'
,
'
.js-show-tab
'
,
this
.
showTab
);
$
(
'
.merge-request-tabs a[data-toggle="tab"]
'
).
off
(
'
click
'
,
this
.
clickTab
);
$
(
'
.merge-request-tabs a[data-toggle="tabvue"]
'
).
off
(
'
click
'
,
this
.
clickTab
);
}
destroyPipelinesView
()
{
...
...
@@ -130,32 +138,57 @@ export default class MergeRequestTabs {
}
}
showTab
(
e
)
{
e
.
preventDefault
();
this
.
activateTab
(
$
(
e
.
target
).
data
(
'
action
'
));
}
clickTab
(
e
)
{
if
(
e
.
currentTarget
&&
isMetaClick
(
e
))
{
const
targetLink
=
e
.
currentTarget
.
getAttribute
(
'
href
'
);
if
(
e
.
currentTarget
)
{
e
.
stopImmediatePropagation
();
e
.
preventDefault
();
const
{
action
}
=
e
.
currentTarget
.
dataset
;
if
(
action
)
{
const
href
=
e
.
currentTarget
.
getAttribute
(
'
href
'
);
this
.
tabShown
(
action
,
href
);
}
else
if
(
isMetaClick
(
e
))
{
const
targetLink
=
e
.
currentTarget
.
getAttribute
(
'
href
'
);
window
.
open
(
targetLink
,
'
_blank
'
);
}
}
}
tabShown
(
e
)
{
const
$target
=
$
(
e
.
target
);
const
action
=
$target
.
data
(
'
action
'
);
tabShown
(
action
,
href
)
{
if
(
action
!==
this
.
currentTab
&&
this
.
mergeRequestTabs
)
{
this
.
currentTab
=
action
;
if
(
this
.
mergeRequestTabPanesAll
)
{
this
.
mergeRequestTabPanesAll
.
forEach
(
el
=>
{
const
tabPane
=
el
;
tabPane
.
style
.
display
=
'
none
'
;
});
}
if
(
this
.
mergeRequestTabsAll
)
{
this
.
mergeRequestTabsAll
.
forEach
(
el
=>
{
el
.
classList
.
remove
(
'
active
'
);
});
}
const
tabPane
=
this
.
mergeRequestTabPanes
.
querySelector
(
`#
${
action
}
`
);
if
(
tabPane
)
tabPane
.
style
.
display
=
'
block
'
;
const
tab
=
this
.
mergeRequestTabs
.
querySelector
(
`.
${
action
}
-tab`
);
if
(
tab
)
tab
.
classList
.
add
(
'
active
'
);
if
(
action
===
'
commits
'
)
{
this
.
loadCommits
(
$target
.
attr
(
'
href
'
));
this
.
loadCommits
(
href
);
this
.
expandView
();
this
.
resetViewContainer
();
this
.
destroyPipelinesView
();
}
else
if
(
action
===
'
new
'
)
{
this
.
expandView
();
this
.
resetViewContainer
();
this
.
destroyPipelinesView
();
}
else
if
(
this
.
isDiffAction
(
action
))
{
if
(
!
isInVueNoteablePage
())
{
this
.
loadDiff
(
$target
.
attr
(
'
href
'
)
);
this
.
loadDiff
(
href
);
}
if
(
bp
.
getBreakpointSize
()
!==
'
lg
'
)
{
this
.
shrinkView
();
...
...
@@ -169,6 +202,9 @@ export default class MergeRequestTabs {
this
.
resetViewContainer
();
this
.
mountPipelinesView
();
}
else
{
this
.
mergeRequestTabPanes
.
querySelector
(
'
#notes
'
).
style
.
display
=
'
block
'
;
this
.
mergeRequestTabs
.
querySelector
(
'
.notes-tab
'
).
classList
.
add
(
'
active
'
);
if
(
bp
.
getBreakpointSize
()
!==
'
xs
'
)
{
this
.
expandView
();
}
...
...
@@ -183,6 +219,7 @@ export default class MergeRequestTabs {
this
.
eventHub
.
$emit
(
'
MergeRequestTabChange
'
,
this
.
getCurrentAction
());
}
}
scrollToElement
(
container
)
{
if
(
location
.
hash
)
{
...
...
@@ -194,12 +231,6 @@ export default class MergeRequestTabs {
}
}
// Activate a tab based on the current action
activateTab
(
action
)
{
// important note: the .tab('show') method triggers 'shown.bs.tab' event itself
$
(
`.merge-request-tabs a[data-action='
${
action
}
']`
).
tab
(
'
show
'
);
}
// Replaces the current Merge Request-specific action in the URL with a new one
//
// If the action is "notes", the URL is reset to the standard
...
...
This diff is collapsed.
Click to expand it.
app/assets/javascripts/notes/components/notes_app.vue
View file @
fb44cb3d
...
...
@@ -175,7 +175,7 @@ export default {
<
template
>
<div
v-
if
=
"shouldShow"
v-
show
=
"shouldShow"
id=
"notes"
>
<ul
...
...
This diff is collapsed.
Click to expand it.
app/helpers/merge_requests_helper.rb
View file @
fb44cb3d
...
...
@@ -108,7 +108,7 @@ module MergeRequestsHelper
data_attrs
=
{
action:
tab
.
to_s
,
target:
"#
#{
tab
}
"
,
toggle:
options
.
fetch
(
:force_link
,
false
)
?
''
:
'tab'
toggle:
options
.
fetch
(
:force_link
,
false
)
?
''
:
'tab
vue
'
}
url
=
case
tab
...
...
This diff is collapsed.
Click to expand it.
app/views/projects/merge_requests/creations/_new_submit.html.haml
View file @
fb44cb3d
...
...
@@ -24,23 +24,28 @@
There are no commits yet.
=
custom_icon
(
'illustration_no_commits'
)
-
else
%ul
.merge-request-tabs.nav.nav-tabs.nav-links.no-top.no-bottom
%li
.commits-tab
=
link_to
url_for
(
safe_params
),
data:
{
target:
'div#commits'
,
action:
'new'
,
toggle:
'tab'
}
do
.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
=
icon
(
'angle-left'
)
.fade-right
=
icon
(
'angle-right'
)
%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
%span
.badge.badge-pill
=
@commits
.
size
-
if
@pipelines
.
any?
%li
.builds-tab
=
link_to
url_for
(
safe_params
.
merge
(
action:
'pipelines'
)),
data:
{
target:
'div#pipelines'
,
action:
'pipelines'
,
toggle:
'tab
'
}
do
=
link_to
url_for
(
safe_params
.
merge
(
action:
'pipelines'
)),
data:
{
target:
'div#pipelines'
,
action:
'pipelines'
,
toggle:
'tabvue
'
}
do
Pipelines
%span
.badge.badge-pill
=
@pipelines
.
size
%li
.diffs-tab
=
link_to
url_for
(
safe_params
.
merge
(
action:
'diffs'
)),
data:
{
target:
'div#diffs'
,
action:
'diffs'
,
toggle:
'tab
'
}
do
=
link_to
url_for
(
safe_params
.
merge
(
action:
'diffs'
)),
data:
{
target:
'div#diffs'
,
action:
'diffs'
,
toggle:
'tabvue
'
}
do
Changes
%span
.badge.badge-pill
=
@merge_request
.
diff_size
.tab-content
#
commits
.commits.tab-pane.active
#diff-notes-app
.tab-content
#
new
.commits.tab-pane.active
=
render
"projects/merge_requests/commits"
#diffs
.diffs.tab-pane
-# This tab is always loaded via AJAX
...
...
This diff is collapsed.
Click to expand it.
spec/helpers/merge_requests_helper_spec.rb
View file @
fb44cb3d
...
...
@@ -54,7 +54,7 @@ describe MergeRequestsHelper do
let
(
:options
)
{
{
force_link:
true
}
}
it
'removes the data-toggle attributes'
do
is_expected
.
not_to
match
(
/data-toggle="tab"/
)
is_expected
.
not_to
match
(
/data-toggle="tab
vue
"/
)
end
end
end
...
...
This diff is collapsed.
Click to expand it.
spec/javascripts/helpers/init_vue_mr_page_helper.js
View file @
fb44cb3d
...
...
@@ -5,12 +5,16 @@ import { userDataMock, notesDataMock, noteableDataMock } from '../notes/mock_dat
import
diffFileMockData
from
'
../diffs/mock_data/diff_file
'
;
export
default
function
initVueMRPage
()
{
const
mrTestEl
=
document
.
createElement
(
'
div
'
);
mrTestEl
.
className
=
'
js-merge-request-test
'
;
document
.
body
.
appendChild
(
mrTestEl
);
const
diffsAppEndpoint
=
'
/diffs/app/endpoint
'
;
const
diffsAppProjectPath
=
'
testproject
'
;
const
mrEl
=
document
.
createElement
(
'
div
'
);
mrEl
.
className
=
'
merge-request fixture-mr
'
;
mrEl
.
setAttribute
(
'
data-mr-action
'
,
'
diffs
'
);
document
.
body
.
appendChild
(
mrEl
);
mrTestEl
.
appendChild
(
mrEl
);
const
mrDiscussionsEl
=
document
.
createElement
(
'
div
'
);
mrDiscussionsEl
.
id
=
'
js-vue-mr-discussions
'
;
...
...
@@ -18,18 +22,18 @@ export default function initVueMRPage() {
mrDiscussionsEl
.
setAttribute
(
'
data-noteable-data
'
,
JSON
.
stringify
(
noteableDataMock
));
mrDiscussionsEl
.
setAttribute
(
'
data-notes-data
'
,
JSON
.
stringify
(
notesDataMock
));
mrDiscussionsEl
.
setAttribute
(
'
data-noteable-type
'
,
'
merge-request
'
);
document
.
body
.
appendChild
(
mrDiscussionsEl
);
mrTestEl
.
appendChild
(
mrDiscussionsEl
);
const
discussionCounterEl
=
document
.
createElement
(
'
div
'
);
discussionCounterEl
.
id
=
'
js-vue-discussion-counter
'
;
document
.
body
.
appendChild
(
discussionCounterEl
);
mrTestEl
.
appendChild
(
discussionCounterEl
);
const
diffsAppEl
=
document
.
createElement
(
'
div
'
);
diffsAppEl
.
id
=
'
js-diffs-app
'
;
diffsAppEl
.
setAttribute
(
'
data-endpoint
'
,
diffsAppEndpoint
);
diffsAppEl
.
setAttribute
(
'
data-project-path
'
,
diffsAppProjectPath
);
diffsAppEl
.
setAttribute
(
'
data-current-user-data
'
,
JSON
.
stringify
(
userDataMock
));
document
.
body
.
appendChild
(
diffsAppEl
);
mrTestEl
.
appendChild
(
diffsAppEl
);
const
mock
=
new
MockAdapter
(
axios
);
mock
.
onGet
(
diffsAppEndpoint
).
reply
(
200
,
{
...
...
This diff is collapsed.
Click to expand it.
spec/javascripts/merge_request_spec.js
View file @
fb44cb3d
...
...
@@ -19,9 +19,11 @@ import IssuablesHelper from '~/helpers/issuables_helper';
spyOn
(
axios
,
'
patch
'
).
and
.
callThrough
();
mock
=
new
MockAdapter
(
axios
);
mock
.
onPatch
(
`
${
gl
.
TEST_HOST
}
/frontend-fixtures/merge-requests-project/merge_requests/1.json`
).
reply
(
200
,
{});
mock
.
onPatch
(
`
${
gl
.
TEST_HOST
}
/frontend-fixtures/merge-requests-project/merge_requests/1.json`
)
.
reply
(
200
,
{});
return
this
.
merge
=
new
MergeRequest
(
);
return
(
this
.
merge
=
new
MergeRequest
()
);
});
afterEach
(()
=>
{
...
...
@@ -32,17 +34,22 @@ import IssuablesHelper from '~/helpers/issuables_helper';
spyOn
(
$
,
'
ajax
'
).
and
.
stub
();
const
changeEvent
=
document
.
createEvent
(
'
HTMLEvents
'
);
changeEvent
.
initEvent
(
'
change
'
,
true
,
true
);
$
(
'
input[type=checkbox]
'
).
attr
(
'
checked
'
,
true
)[
0
].
dispatchEvent
(
changeEvent
);
$
(
'
input[type=checkbox]
'
)
.
attr
(
'
checked
'
,
true
)[
0
]
.
dispatchEvent
(
changeEvent
);
return
expect
(
$
(
'
.js-task-list-field
'
).
val
()).
toBe
(
'
- [x] Task List Item
'
);
});
it
(
'
submits an ajax request on tasklist:changed
'
,
(
done
)
=>
{
it
(
'
submits an ajax request on tasklist:changed
'
,
done
=>
{
$
(
'
.js-task-list-field
'
).
trigger
(
'
tasklist:changed
'
);
setTimeout
(()
=>
{
expect
(
axios
.
patch
).
toHaveBeenCalledWith
(
`
${
gl
.
TEST_HOST
}
/frontend-fixtures/merge-requests-project/merge_requests/1.json`
,
{
expect
(
axios
.
patch
).
toHaveBeenCalledWith
(
`
${
gl
.
TEST_HOST
}
/frontend-fixtures/merge-requests-project/merge_requests/1.json`
,
{
merge_request
:
{
description
:
'
- [ ] Task List Item
'
},
});
},
);
done
();
});
});
...
...
@@ -119,4 +126,4 @@ import IssuablesHelper from '~/helpers/issuables_helper';
});
});
});
}
).
call
(
window
);
}
.
call
(
window
)
);
This diff is collapsed.
Click to expand it.
spec/javascripts/merge_request_tabs_spec.js
View file @
fb44cb3d
...
...
@@ -40,6 +40,7 @@ describe('MergeRequestTabs', function() {
this
.
class
.
unbindEvents
();
this
.
class
.
destroyPipelinesView
();
mrPageMock
.
restore
();
$
(
'
.js-merge-request-test
'
).
remove
();
});
describe
(
'
opensInNewTab
'
,
function
()
{
...
...
This diff is collapsed.
Click to expand it.
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