diff options
| -rw-r--r-- | app/assets/javascripts/reports/components/issues_list.vue | 85 | ||||
| -rw-r--r-- | app/assets/javascripts/reports/components/report_item.vue (renamed from app/assets/javascripts/reports/components/report_issues.vue) | 44 | ||||
| -rw-r--r-- | app/assets/javascripts/vue_shared/components/smart_virtual_list.vue | 42 | ||||
| -rw-r--r-- | changelogs/unreleased/7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee.yml | 5 | ||||
| -rw-r--r-- | spec/javascripts/reports/components/grouped_test_reports_app_spec.js | 4 | ||||
| -rw-r--r-- | spec/javascripts/reports/components/report_section_spec.js | 2 | ||||
| -rw-r--r-- | spec/javascripts/vue_shared/components/smart_virtual_list_spec.js | 83 |
7 files changed, 197 insertions, 68 deletions
diff --git a/app/assets/javascripts/reports/components/issues_list.vue b/app/assets/javascripts/reports/components/issues_list.vue index 3b425ee2fed..f4243522ef8 100644 --- a/app/assets/javascripts/reports/components/issues_list.vue +++ b/app/assets/javascripts/reports/components/issues_list.vue @@ -1,18 +1,31 @@ <script> -import IssuesBlock from '~/reports/components/report_issues.vue'; -import { STATUS_SUCCESS, STATUS_FAILED, STATUS_NEUTRAL } from '~/reports/constants'; +import ReportItem from '~/reports/components/report_item.vue'; +import { STATUS_FAILED, STATUS_NEUTRAL, STATUS_SUCCESS } from '~/reports/constants'; +import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue'; + +const wrapIssueWithState = (status, isNew = false) => issue => ({ + status: issue.status || status, + isNew, + issue, +}); /** * Renders block of issues */ - export default { components: { - IssuesBlock, + SmartVirtualList, + ReportItem, }, - success: STATUS_SUCCESS, - failed: STATUS_FAILED, - neutral: STATUS_NEUTRAL, + // Typical height of a report item in px + typicalReportItemHeight: 32, + /* + The maximum amount of shown issues. This is calculated by + ( max-height of report-block-list / typicalReportItemHeight ) + some safety margin + We will use VirtualList if we have more items than this number. + For entries lower than this number, the virtual scroll list calculates the total height of the element wrongly. + */ + maxShownReportItems: 20, props: { newIssues: { type: Array, @@ -40,42 +53,34 @@ export default { default: '', }, }, + computed: { + issuesWithState() { + return [ + ...this.newIssues.map(wrapIssueWithState(STATUS_FAILED, true)), + ...this.unresolvedIssues.map(wrapIssueWithState(STATUS_FAILED)), + ...this.neutralIssues.map(wrapIssueWithState(STATUS_NEUTRAL)), + ...this.resolvedIssues.map(wrapIssueWithState(STATUS_SUCCESS)), + ]; + }, + }, }; </script> <template> - <div class="report-block-container"> - - <issues-block - v-if="newIssues.length" - :component="component" - :issues="newIssues" - class="js-mr-code-new-issues" - status="failed" - is-new - /> - - <issues-block - v-if="unresolvedIssues.length" - :component="component" - :issues="unresolvedIssues" - :status="$options.failed" - class="js-mr-code-new-issues" - /> - - <issues-block - v-if="neutralIssues.length" - :component="component" - :issues="neutralIssues" - :status="$options.neutral" - class="js-mr-code-non-issues" - /> - - <issues-block - v-if="resolvedIssues.length" + <smart-virtual-list + :length="issuesWithState.length" + :remain="$options.maxShownReportItems" + :size="$options.typicalReportItemHeight" + class="report-block-container" + wtag="ul" + wclass="report-block-list" + > + <report-item + v-for="(wrapped, index) in issuesWithState" + :key="index" + :issue="wrapped.issue" + :status="wrapped.status" :component="component" - :issues="resolvedIssues" - :status="$options.success" - class="js-mr-code-resolved-issues" + :is-new="wrapped.isNew" /> - </div> + </smart-virtual-list> </template> diff --git a/app/assets/javascripts/reports/components/report_issues.vue b/app/assets/javascripts/reports/components/report_item.vue index a2a03945ae3..01e6d357a21 100644 --- a/app/assets/javascripts/reports/components/report_issues.vue +++ b/app/assets/javascripts/reports/components/report_item.vue @@ -3,14 +3,14 @@ import IssueStatusIcon from '~/reports/components/issue_status_icon.vue'; import { components, componentNames } from '~/reports/components/issue_body'; export default { - name: 'ReportIssues', + name: 'ReportItem', components: { IssueStatusIcon, ...components, }, props: { - issues: { - type: Array, + issue: { + type: Object, required: true, }, component: { @@ -33,27 +33,21 @@ export default { }; </script> <template> - <div> - <ul class="report-block-list"> - <li - v-for="(issue, index) in issues" - :key="index" - :class="{ 'is-dismissed': issue.isDismissed }" - class="report-block-list-issue" - > - <issue-status-icon - :status="issue.status || status" - class="append-right-5" - /> + <li + :class="{ 'is-dismissed': issue.isDismissed }" + class="report-block-list-issue" + > + <issue-status-icon + :status="status" + class="append-right-5" + /> - <component - :is="component" - v-if="component" - :issue="issue" - :status="issue.status || status" - :is-new="isNew" - /> - </li> - </ul> - </div> + <component + :is="component" + v-if="component" + :issue="issue" + :status="status" + :is-new="isNew" + /> + </li> </template> diff --git a/app/assets/javascripts/vue_shared/components/smart_virtual_list.vue b/app/assets/javascripts/vue_shared/components/smart_virtual_list.vue new file mode 100644 index 00000000000..63034a45f77 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/smart_virtual_list.vue @@ -0,0 +1,42 @@ +<script> +import VirtualList from 'vue-virtual-scroll-list'; + +export default { + name: 'SmartVirtualList', + components: { VirtualList }, + props: { + size: { type: Number, required: true }, + length: { type: Number, required: true }, + remain: { type: Number, required: true }, + rtag: { type: String, default: 'div' }, + wtag: { type: String, default: 'div' }, + wclass: { type: String, default: null }, + }, +}; +</script> +<template> + <virtual-list + v-if="length > remain" + v-bind="$attrs" + :size="remain" + :remain="remain" + :rtag="rtag" + :wtag="wtag" + :wclass="wclass" + class="js-virtual-list" + > + <slot></slot> + </virtual-list> + <component + :is="rtag" + v-else + class="js-plain-element" + > + <component + :is="wtag" + :class="wclass" + > + <slot></slot> + </component> + </component> +</template> diff --git a/changelogs/unreleased/7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee.yml b/changelogs/unreleased/7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee.yml new file mode 100644 index 00000000000..aaae8feb220 --- /dev/null +++ b/changelogs/unreleased/7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of rendering large reports +merge_request: 22835 +author: +type: performance diff --git a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js index f58515daa4f..69767d9cf1c 100644 --- a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js +++ b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js @@ -151,11 +151,11 @@ describe('Grouped Test Reports App', () => { it('renders resolved failures', done => { setTimeout(() => { - expect(vm.$el.querySelector('.js-mr-code-resolved-issues').textContent).toContain( + expect(vm.$el.querySelector('.report-block-container').textContent).toContain( resolvedFailures.suites[0].resolved_failures[0].name, ); - expect(vm.$el.querySelector('.js-mr-code-resolved-issues').textContent).toContain( + expect(vm.$el.querySelector('.report-block-container').textContent).toContain( resolvedFailures.suites[0].resolved_failures[1].name, ); done(); diff --git a/spec/javascripts/reports/components/report_section_spec.js b/spec/javascripts/reports/components/report_section_spec.js index eb7307605d7..b02af8baaec 100644 --- a/spec/javascripts/reports/components/report_section_spec.js +++ b/spec/javascripts/reports/components/report_section_spec.js @@ -120,7 +120,7 @@ describe('Report section', () => { 'Code quality improved on 1 point and degraded on 1 point', ); - expect(vm.$el.querySelectorAll('.js-mr-code-resolved-issues li').length).toEqual( + expect(vm.$el.querySelectorAll('.report-block-container li').length).toEqual( resolvedIssues.length, ); }); diff --git a/spec/javascripts/vue_shared/components/smart_virtual_list_spec.js b/spec/javascripts/vue_shared/components/smart_virtual_list_spec.js new file mode 100644 index 00000000000..e723fead65e --- /dev/null +++ b/spec/javascripts/vue_shared/components/smart_virtual_list_spec.js @@ -0,0 +1,83 @@ +import Vue from 'vue'; +import SmartVirtualScrollList from '~/vue_shared/components/smart_virtual_list.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +describe('Toggle Button', () => { + let vm; + + const createComponent = ({ length, remain }) => { + const smartListProperties = { + rtag: 'section', + wtag: 'ul', + wclass: 'test-class', + // Size in pixels does not matter for our tests here + size: 35, + length, + remain, + }; + + const Component = Vue.extend({ + components: { + SmartVirtualScrollList, + }, + smartListProperties, + items: Array(length).fill(1), + template: ` + <smart-virtual-scroll-list v-bind="$options.smartListProperties"> + <li v-for="(val, key) in $options.items" :key="key">{{ key + 1 }}</li> + </smart-virtual-scroll-list>`, + }); + + return mountComponent(Component); + }; + + afterEach(() => { + vm.$destroy(); + }); + + describe('if the list is shorter than the maximum shown elements', () => { + const listLength = 10; + + beforeEach(() => { + vm = createComponent({ length: listLength, remain: 20 }); + }); + + it('renders without the vue-virtual-scroll-list component', () => { + expect(vm.$el.classList).not.toContain('js-virtual-list'); + expect(vm.$el.classList).toContain('js-plain-element'); + }); + + it('renders list with provided tags and classes for the wrapper elements', () => { + expect(vm.$el.tagName).toEqual('SECTION'); + expect(vm.$el.firstChild.tagName).toEqual('UL'); + expect(vm.$el.firstChild.classList).toContain('test-class'); + }); + + it('renders all children list elements', () => { + expect(vm.$el.querySelectorAll('li').length).toEqual(listLength); + }); + }); + + describe('if the list is longer than the maximum shown elements', () => { + const maxItemsShown = 20; + + beforeEach(() => { + vm = createComponent({ length: 1000, remain: maxItemsShown }); + }); + + it('uses the vue-virtual-scroll-list component', () => { + expect(vm.$el.classList).toContain('js-virtual-list'); + expect(vm.$el.classList).not.toContain('js-plain-element'); + }); + + it('renders list with provided tags and classes for the wrapper elements', () => { + expect(vm.$el.tagName).toEqual('SECTION'); + expect(vm.$el.firstChild.tagName).toEqual('UL'); + expect(vm.$el.firstChild.classList).toContain('test-class'); + }); + + it('renders at max twice the maximum shown elements', () => { + expect(vm.$el.querySelectorAll('li').length).toBeLessThanOrEqual(2 * maxItemsShown); + }); + }); +}); |