From 5a95d6f8dae00b31b694759c6ddbf6d83b1a7890 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 10 May 2017 12:29:33 +0100 Subject: [PATCH 1/7] Refactored issue tealtime elements This is to match our docs better and will also help a future issue. Also made it possible for the description & title to be readable when JS is disabled --- .../javascripts/issue_show/actions/tasks.js | 27 --- .../javascripts/issue_show/components/app.vue | 95 +++++++++ .../issue_show/components/description.vue | 100 ++++++++++ .../issue_show/components/title.vue | 53 ++++++ app/assets/javascripts/issue_show/index.js | 34 ++-- .../issue_show/issue_title_description.vue | 180 ------------------ .../javascripts/issue_show/mixins/animate.js | 13 ++ .../javascripts/issue_show/services/index.js | 14 +- .../javascripts/issue_show/stores/index.js | 25 +++ app/views/projects/issues/show.html.haml | 11 +- 10 files changed, 327 insertions(+), 225 deletions(-) delete mode 100644 app/assets/javascripts/issue_show/actions/tasks.js create mode 100644 app/assets/javascripts/issue_show/components/app.vue create mode 100644 app/assets/javascripts/issue_show/components/description.vue create mode 100644 app/assets/javascripts/issue_show/components/title.vue delete mode 100644 app/assets/javascripts/issue_show/issue_title_description.vue create mode 100644 app/assets/javascripts/issue_show/mixins/animate.js create mode 100644 app/assets/javascripts/issue_show/stores/index.js diff --git a/app/assets/javascripts/issue_show/actions/tasks.js b/app/assets/javascripts/issue_show/actions/tasks.js deleted file mode 100644 index 0740a9f559c..00000000000 --- a/app/assets/javascripts/issue_show/actions/tasks.js +++ /dev/null @@ -1,27 +0,0 @@ -export default (newStateData, tasks) => { - const $tasks = $('#task_status'); - const $tasksShort = $('#task_status_short'); - const $issueableHeader = $('.issuable-header'); - const tasksStates = { newState: null, currentState: null }; - - if ($tasks.length === 0) { - if (!(newStateData.task_status.indexOf('0 of 0') === 0)) { - $issueableHeader.append(`${newStateData.task_status}`); - } else { - $issueableHeader.append(''); - } - } else { - tasksStates.newState = newStateData.task_status.indexOf('0 of 0') === 0; - tasksStates.currentState = tasks.indexOf('0 of 0') === 0; - } - - if ($tasks.length !== 0 && !tasksStates.newState) { - $tasks.text(newStateData.task_status); - $tasksShort.text(newStateData.task_status); - } else if (tasksStates.currentState) { - $issueableHeader.append(`${newStateData.task_status}`); - } else if (tasksStates.newState) { - $tasks.remove(); - $tasksShort.remove(); - } -}; diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue new file mode 100644 index 00000000000..752d07f7ef0 --- /dev/null +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -0,0 +1,95 @@ + + + diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue new file mode 100644 index 00000000000..298f87b6d22 --- /dev/null +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -0,0 +1,100 @@ + + + diff --git a/app/assets/javascripts/issue_show/components/title.vue b/app/assets/javascripts/issue_show/components/title.vue new file mode 100644 index 00000000000..a9dabd4cff1 --- /dev/null +++ b/app/assets/javascripts/issue_show/components/title.vue @@ -0,0 +1,53 @@ + + + diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index eb20a597bb5..af11ae4c533 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/app/assets/javascripts/issue_show/index.js @@ -1,20 +1,32 @@ import Vue from 'vue'; -import IssueTitle from './issue_title_description.vue'; +import issuableApp from './components/app.vue'; import '../vue_shared/vue_resource_interceptor'; -(() => { - const issueTitleData = document.querySelector('.issue-title-data').dataset; - const { canUpdateTasksClass, endpoint } = issueTitleData; +document.addEventListener('DOMContentLoaded', () => { + const issuableElement = document.getElementById('js-issuable-app'); + const issuableTitleElement = issuableElement.querySelector('.title'); + const issuableDescriptionElement = issuableElement.querySelector('.wiki'); + const issuableDescriptionTextarea = issuableElement.querySelector('.js-task-list-field'); + const { + canUpdate, + endpoint, + issuableRef, + } = issuableElement.dataset; - const vm = new Vue({ - el: '.issue-title-entrypoint', - render: createElement => createElement(IssueTitle, { + return new Vue({ + el: issuableElement, + components: { + issuableApp, + }, + render: createElement => createElement('issuable-app', { props: { - canUpdateTasksClass, + canUpdate: gl.utils.convertPermissionToBoolean(canUpdate), endpoint, + issuableRef, + initialTitle: issuableTitleElement.innerHTML, + initialDescriptionHtml: issuableDescriptionElement ? issuableDescriptionElement.innerHTML : '', + initialDescriptionText: issuableDescriptionTextarea ? issuableDescriptionTextarea.textContent : '', }, }), }); - - return vm; -})(); +}); diff --git a/app/assets/javascripts/issue_show/issue_title_description.vue b/app/assets/javascripts/issue_show/issue_title_description.vue deleted file mode 100644 index dc3ba2550c5..00000000000 --- a/app/assets/javascripts/issue_show/issue_title_description.vue +++ /dev/null @@ -1,180 +0,0 @@ - - - diff --git a/app/assets/javascripts/issue_show/mixins/animate.js b/app/assets/javascripts/issue_show/mixins/animate.js new file mode 100644 index 00000000000..eda6302aa8b --- /dev/null +++ b/app/assets/javascripts/issue_show/mixins/animate.js @@ -0,0 +1,13 @@ +export default { + methods: { + animateChange() { + this.preAnimation = true; + this.pulseAnimation = false; + + this.$nextTick(() => { + this.preAnimation = false; + this.pulseAnimation = true; + }); + }, + }, +}; diff --git a/app/assets/javascripts/issue_show/services/index.js b/app/assets/javascripts/issue_show/services/index.js index c4ab0b1e07a..348ad8d6813 100644 --- a/app/assets/javascripts/issue_show/services/index.js +++ b/app/assets/javascripts/issue_show/services/index.js @@ -1,10 +1,16 @@ +import Vue from 'vue'; +import VueResource from 'vue-resource'; + +Vue.use(VueResource); + export default class Service { - constructor(resource, endpoint) { - this.resource = resource; + constructor(endpoint) { this.endpoint = endpoint; + + this.resource = Vue.resource(this.endpoint); } - getTitle() { - return this.resource.get(this.endpoint); + getData() { + return this.resource.get(); } } diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js new file mode 100644 index 00000000000..9c759dc53cb --- /dev/null +++ b/app/assets/javascripts/issue_show/stores/index.js @@ -0,0 +1,25 @@ +export default class Store { + constructor({ + title, + descriptionHtml, + descriptionText, + }) { + this.state = { + titleHtml: title, + titleText: '', + descriptionHtml, + descriptionText, + taskStatus: '', + updatedAt: '', + }; + } + + updateState(data) { + this.state.titleHtml = data.title; + this.state.titleText = data.title_text; + this.state.descriptionHtml = data.description; + this.state.descriptionText = data.description_text; + this.state.taskStatus = data.task_status; + this.state.updatedAt = data.updated_at; + } +} diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 9084883eb3e..bd03593eb98 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -51,10 +51,15 @@ .issue-details.issuable-details .detail-page-description.content-block - .issue-title-data.hidden{ "data" => { "endpoint" => rendered_title_namespace_project_issue_path(@project.namespace, @project, @issue), - "can-update-tasks-class" => can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '', + #js-issuable-app{ "data" => { "endpoint" => rendered_title_namespace_project_issue_path(@project.namespace, @project, @issue), + "can-update" => can?(current_user, :update_issue, @issue).to_s, + "issuable-ref" => @issue.to_reference, } } - .issue-title-entrypoint + %h2.title= markdown_field(@issue, :title) + - if @issue.description.present? + .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' } + .wiki= markdown_field(@issue, :description) + %textarea.hidden.js-task-list-field= @issue.description = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') From 5a5b06de3eb9c9607c31d9e788fec33b2ee8078e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 10 May 2017 12:59:37 +0100 Subject: [PATCH 2/7] Fixed task status with mobile --- .../issue_show/components/description.vue | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 298f87b6d22..1f594e0d5ea 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -56,19 +56,25 @@ }); }, taskStatus() { - const $issueableHeader = $('.issuable-header'); + const taskRegexMatches = this.taskStatus.match(/(\d+) of (\d+)/); + const $issueableHeader = $('.issuable-meta'); let $tasks = $('#task_status'); let $tasksShort = $('#task_status_short'); - if (this.taskStatus.indexOf('0 of 0') >= 0) { + if (this.taskStatus.indexOf('0 of 0') >= 0 || this.taskStatus.trim() === '') { $tasks.remove(); $tasksShort.remove(); } else if (!$tasks.length && !$tasksShort.length) { - $tasks = $issueableHeader.append(''); - $tasksShort = $issueableHeader.append(''); + $tasks = $issueableHeader.append('') + .find('#task_status'); + $tasksShort = $issueableHeader.append('') + .find('#task_status_short'); } - $tasks.text(this.taskStatus); + if (taskRegexMatches) { + $tasks.text(this.taskStatus); + $tasksShort.text(`${taskRegexMatches[1]}/${taskRegexMatches[2]} task${taskRegexMatches[2] > 1 ? 's' : ''}`); + } }, }, }; From 3313df580c79435187ba673499b48ba3be783cea Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 10 May 2017 15:05:58 +0100 Subject: [PATCH 3/7] Added a bunch of specs for the different components --- .../issue_show/components/description.vue | 10 +- .../app_spec.js} | 46 ++++----- .../issue_show/components/description_spec.js | 98 +++++++++++++++++++ .../issue_show/components/title_spec.js | 67 +++++++++++++ 4 files changed, 193 insertions(+), 28 deletions(-) rename spec/javascripts/issue_show/{issue_title_description_spec.js => components/app_spec.js} (51%) create mode 100644 spec/javascripts/issue_show/components/description_spec.js create mode 100644 spec/javascripts/issue_show/components/title_spec.js diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 1f594e0d5ea..18e73960e34 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -57,17 +57,17 @@ }, taskStatus() { const taskRegexMatches = this.taskStatus.match(/(\d+) of (\d+)/); - const $issueableHeader = $('.issuable-meta'); - let $tasks = $('#task_status'); - let $tasksShort = $('#task_status_short'); + const $issuableHeader = $('.issuable-meta'); + let $tasks = $('#task_status', $issuableHeader); + let $tasksShort = $('#task_status_short', $issuableHeader); if (this.taskStatus.indexOf('0 of 0') >= 0 || this.taskStatus.trim() === '') { $tasks.remove(); $tasksShort.remove(); } else if (!$tasks.length && !$tasksShort.length) { - $tasks = $issueableHeader.append('') + $tasks = $issuableHeader.append('') .find('#task_status'); - $tasksShort = $issueableHeader.append('') + $tasksShort = $issuableHeader.append('') .find('#task_status_short'); } diff --git a/spec/javascripts/issue_show/issue_title_description_spec.js b/spec/javascripts/issue_show/components/app_spec.js similarity index 51% rename from spec/javascripts/issue_show/issue_title_description_spec.js rename to spec/javascripts/issue_show/components/app_spec.js index 1ec4fe58b08..c33321814a5 100644 --- a/spec/javascripts/issue_show/issue_title_description_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -1,11 +1,8 @@ import Vue from 'vue'; -import $ from 'jquery'; import '~/render_math'; import '~/render_gfm'; -import issueTitleDescription from '~/issue_show/issue_title_description.vue'; -import issueShowData from './mock_data'; - -window.$ = $; +import issuableApp from '~/issue_show/components/app.vue'; +import issueShowData from '../mock_data'; const issueShowInterceptor = data => (request, next) => { next(request.respondWith(JSON.stringify(data), { @@ -16,13 +13,25 @@ const issueShowInterceptor = data => (request, next) => { })); }; -describe('Issue Title', () => { +describe('Issuable output', () => { document.body.innerHTML = ''; - let IssueTitleDescriptionComponent; + let vm; beforeEach(() => { - IssueTitleDescriptionComponent = Vue.extend(issueTitleDescription); + const IssuableDescriptionComponent = Vue.extend(issuableApp); + Vue.http.interceptors.push(issueShowInterceptor(issueShowData.initialRequest)); + + vm = new IssuableDescriptionComponent({ + propsData: { + canUpdate: true, + endpoint: '/gitlab-org/gitlab-shell/issues/9/rendered_title', + issuableRef: '#1', + initialTitle: '', + initialDescriptionHtml: '', + initialDescriptionText: '', + }, + }).$mount(); }); afterEach(() => { @@ -30,28 +39,19 @@ describe('Issue Title', () => { }); it('should render a title/description and update title/description on update', (done) => { - Vue.http.interceptors.push(issueShowInterceptor(issueShowData.initialRequest)); - - const issueShowComponent = new IssueTitleDescriptionComponent({ - propsData: { - canUpdateIssue: '.css-stuff', - endpoint: '/gitlab-org/gitlab-shell/issues/9/rendered_title', - }, - }).$mount(); - setTimeout(() => { expect(document.querySelector('title').innerText).toContain('this is a title (#1)'); - expect(issueShowComponent.$el.querySelector('.title').innerHTML).toContain('

this is a title

'); - expect(issueShowComponent.$el.querySelector('.wiki').innerHTML).toContain('

this is a description!

'); - expect(issueShowComponent.$el.querySelector('.js-task-list-field').innerText).toContain('this is a description'); + expect(vm.$el.querySelector('.title').innerHTML).toContain('

this is a title

'); + expect(vm.$el.querySelector('.wiki').innerHTML).toContain('

this is a description!

'); + expect(vm.$el.querySelector('.js-task-list-field').innerText).toContain('this is a description'); Vue.http.interceptors.push(issueShowInterceptor(issueShowData.secondRequest)); setTimeout(() => { expect(document.querySelector('title').innerText).toContain('2 (#1)'); - expect(issueShowComponent.$el.querySelector('.title').innerHTML).toContain('

2

'); - expect(issueShowComponent.$el.querySelector('.wiki').innerHTML).toContain('

42

'); - expect(issueShowComponent.$el.querySelector('.js-task-list-field').innerText).toContain('42'); + expect(vm.$el.querySelector('.title').innerHTML).toContain('

2

'); + expect(vm.$el.querySelector('.wiki').innerHTML).toContain('

42

'); + expect(vm.$el.querySelector('.js-task-list-field').innerText).toContain('42'); done(); }); diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js new file mode 100644 index 00000000000..df9941a2bdb --- /dev/null +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -0,0 +1,98 @@ +import Vue from 'vue'; +import descriptionComponent from '~/issue_show/components/description.vue'; + +describe('Description component', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(descriptionComponent); + + if (!document.querySelector('.issuable-meta')) { + const metaData = document.createElement('div'); + metaData.classList.add('issuable-meta'); + + document.body.appendChild(metaData); + } + + vm = new Component({ + propsData: { + canUpdate: true, + descriptionHtml: 'test', + descriptionText: 'test', + updatedAt: new Date().toString(), + taskStatus: '', + }, + }).$mount(); + }); + + it('animates description changes', (done) => { + vm.descriptionHtml = 'changed'; + + Vue.nextTick(() => { + expect( + vm.$el.querySelector('.wiki').classList.contains('issue-realtime-pre-pulse'), + ).toBeTruthy(); + + setTimeout(() => { + expect( + vm.$el.querySelector('.wiki').classList.contains('issue-realtime-trigger-pulse'), + ).toBeTruthy(); + + done(); + }); + }); + }); + + it('re-inits the TaskList when description changed', (done) => { + spyOn(gl, 'TaskList'); + vm.descriptionHtml = 'changed'; + + setTimeout(() => { + expect( + gl.TaskList, + ).toHaveBeenCalled(); + + done(); + }); + }); + + it('does not re-init the TaskList when canUpdate is false', (done) => { + spyOn(gl, 'TaskList'); + vm.canUpdate = false; + vm.descriptionHtml = 'changed'; + + setTimeout(() => { + expect( + gl.TaskList, + ).not.toHaveBeenCalled(); + + done(); + }); + }); + + describe('taskStatus', () => { + it('adds full taskStatus', (done) => { + vm.taskStatus = '1 of 1'; + + setTimeout(() => { + expect( + document.querySelector('.issuable-meta #task_status').textContent.trim(), + ).toBe('1 of 1'); + + done(); + }); + }); + + it('adds short taskStatus', (done) => { + vm.taskStatus = '1 of 1'; + + setTimeout(() => { + expect( + document.querySelector('.issuable-meta #task_status_short').textContent.trim(), + ).toBe('1/1 task'); + + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/issue_show/components/title_spec.js b/spec/javascripts/issue_show/components/title_spec.js new file mode 100644 index 00000000000..2f953e7e92e --- /dev/null +++ b/spec/javascripts/issue_show/components/title_spec.js @@ -0,0 +1,67 @@ +import Vue from 'vue'; +import titleComponent from '~/issue_show/components/title.vue'; + +describe('Title component', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(titleComponent); + vm = new Component({ + propsData: { + issuableRef: '#1', + titleHtml: 'Testing ', + titleText: 'Testing', + }, + }).$mount(); + }); + + it('renders title HTML', () => { + expect( + vm.$el.innerHTML.trim(), + ).toBe('Testing '); + }); + + it('updates page title when changing titleHtml', (done) => { + spyOn(vm, 'setPageTitle'); + vm.titleHtml = 'test'; + + Vue.nextTick(() => { + expect( + vm.setPageTitle, + ).toHaveBeenCalled(); + + done(); + }); + }); + + it('animates title changes', (done) => { + vm.titleHtml = 'test'; + + Vue.nextTick(() => { + expect( + vm.$el.classList.contains('issue-realtime-pre-pulse'), + ).toBeTruthy(); + + setTimeout(() => { + expect( + vm.$el.classList.contains('issue-realtime-trigger-pulse'), + ).toBeTruthy(); + + done(); + }); + }); + }); + + it('updates page title after changing title', (done) => { + vm.titleHtml = 'changed'; + vm.titleText = 'changed'; + + Vue.nextTick(() => { + expect( + document.querySelector('title').textContent.trim(), + ).toContain('changed'); + + done(); + }); + }); +}); From 598b05dfff2fa16f9dc835c3f2cea88da741a9d7 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 10 May 2017 17:28:36 +0100 Subject: [PATCH 4/7] Fixed a bunch of errors with invalid prop Use v-model on textrea --- .../javascripts/issue_show/components/app.vue | 3 +- .../issue_show/components/description.vue | 38 +++++++++++-------- .../javascripts/issue_show/stores/index.js | 4 +- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 752d07f7ef0..770a0dcd27e 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -37,7 +37,7 @@ export default { }, data() { const store = new Store({ - title: this.initialTitle, + titleHtml: this.initialTitle, descriptionHtml: this.initialDescriptionHtml, descriptionText: this.initialDescriptionText, }); @@ -86,6 +86,7 @@ export default { :title-html="state.titleHtml" :title-text="state.titleText" /> diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js index 9c759dc53cb..8e89a2b7730 100644 --- a/app/assets/javascripts/issue_show/stores/index.js +++ b/app/assets/javascripts/issue_show/stores/index.js @@ -1,11 +1,11 @@ export default class Store { constructor({ - title, + titleHtml, descriptionHtml, descriptionText, }) { this.state = { - titleHtml: title, + titleHtml, titleText: '', descriptionHtml, descriptionText, From 584ea586ff04bd57852748f4e6b046200eac7f68 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 10 May 2017 18:50:14 +0100 Subject: [PATCH 5/7] Fixed tests on textarea looking for innerText instead of value --- app/controllers/projects/issues_controller.rb | 1 - spec/javascripts/issue_show/components/app_spec.js | 6 +++--- spec/javascripts/issue_show/mock_data.js | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 58d41e0478d..c9cd42cf99d 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -208,7 +208,6 @@ class Projects::IssuesController < Projects::ApplicationController description: view_context.markdown_field(@issue, :description), description_text: @issue.description, task_status: @issue.task_status, - issue_number: @issue.iid, updated_at: @issue.updated_at, } end diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index c33321814a5..2c294eb4789 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -13,7 +13,7 @@ const issueShowInterceptor = data => (request, next) => { })); }; -describe('Issuable output', () => { +fdescribe('Issuable output', () => { document.body.innerHTML = ''; let vm; @@ -43,7 +43,7 @@ describe('Issuable output', () => { expect(document.querySelector('title').innerText).toContain('this is a title (#1)'); expect(vm.$el.querySelector('.title').innerHTML).toContain('

this is a title

'); expect(vm.$el.querySelector('.wiki').innerHTML).toContain('

this is a description!

'); - expect(vm.$el.querySelector('.js-task-list-field').innerText).toContain('this is a description'); + expect(vm.$el.querySelector('.js-task-list-field').value).toContain('this is a description'); Vue.http.interceptors.push(issueShowInterceptor(issueShowData.secondRequest)); @@ -51,7 +51,7 @@ describe('Issuable output', () => { expect(document.querySelector('title').innerText).toContain('2 (#1)'); expect(vm.$el.querySelector('.title').innerHTML).toContain('

2

'); expect(vm.$el.querySelector('.wiki').innerHTML).toContain('

42

'); - expect(vm.$el.querySelector('.js-task-list-field').innerText).toContain('42'); + expect(vm.$el.querySelector('.js-task-list-field').value).toContain('42'); done(); }); diff --git a/spec/javascripts/issue_show/mock_data.js b/spec/javascripts/issue_show/mock_data.js index ad5a7b63470..6683d581bc5 100644 --- a/spec/javascripts/issue_show/mock_data.js +++ b/spec/javascripts/issue_show/mock_data.js @@ -4,23 +4,23 @@ export default { title_text: 'this is a title', description: '

this is a description!

', description_text: 'this is a description', - issue_number: 1, task_status: '2 of 4 completed', + updated_at: new Date().toString(), }, secondRequest: { title: '

2

', title_text: '2', description: '

42

', description_text: '42', - issue_number: 1, task_status: '0 of 0 completed', + updated_at: new Date().toString(), }, issueSpecRequest: { title: '

this is a title

', title_text: 'this is a title', description: '
  • Task List Item
  • ', description_text: '- [ ] Task List Item', - issue_number: 1, task_status: '0 of 1 completed', + updated_at: new Date().toString(), }, }; From 0c55c8891b891eadef41026420f955bb504f305f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 11 May 2017 12:25:22 +0100 Subject: [PATCH 6/7] Remove some weird code to add/remove the task status Moved the data into the data method Renamed edited ago class name --- .../issue_show/components/description.vue | 19 ++---- app/assets/javascripts/issue_show/index.js | 60 +++++++++++-------- app/assets/stylesheets/framework/mobile.scss | 2 +- app/helpers/issuables_helper.rb | 8 +-- app/views/projects/issues/show.html.haml | 2 +- 5 files changed, 46 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 770bf68e093..4ad3eb7dfd7 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -29,7 +29,7 @@ return { preAnimation: false, pulseAnimation: false, - timeAgoEl: $('.issue_edited_ago'), + timeAgoEl: $('.js-issue-edited-ago'), }; }, watch: { @@ -49,22 +49,15 @@ taskStatus() { const taskRegexMatches = this.taskStatus.match(/(\d+) of (\d+)/); const $issuableHeader = $('.issuable-meta'); - let $tasks = $('#task_status', $issuableHeader); - let $tasksShort = $('#task_status_short', $issuableHeader); - - if (this.taskStatus.indexOf('0 of 0') >= 0 || this.taskStatus.trim() === '') { - $tasks.remove(); - $tasksShort.remove(); - } else if (!$tasks.length && !$tasksShort.length) { - $tasks = $issuableHeader.append('') - .find('#task_status'); - $tasksShort = $issuableHeader.append('') - .find('#task_status_short'); - } + const $tasks = $('#task_status', $issuableHeader); + const $tasksShort = $('#task_status_short', $issuableHeader); if (taskRegexMatches) { $tasks.text(this.taskStatus); $tasksShort.text(`${taskRegexMatches[1]}/${taskRegexMatches[2]} task${taskRegexMatches[2] > 1 ? 's' : ''}`); + } else { + $tasks.text(''); + $tasksShort.text(''); } }, }, diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index af11ae4c533..f06e33dee60 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/app/assets/javascripts/issue_show/index.js @@ -2,31 +2,41 @@ import Vue from 'vue'; import issuableApp from './components/app.vue'; import '../vue_shared/vue_resource_interceptor'; -document.addEventListener('DOMContentLoaded', () => { - const issuableElement = document.getElementById('js-issuable-app'); - const issuableTitleElement = issuableElement.querySelector('.title'); - const issuableDescriptionElement = issuableElement.querySelector('.wiki'); - const issuableDescriptionTextarea = issuableElement.querySelector('.js-task-list-field'); - const { - canUpdate, - endpoint, - issuableRef, - } = issuableElement.dataset; +document.addEventListener('DOMContentLoaded', () => new Vue({ + el: document.getElementById('js-issuable-app'), + components: { + issuableApp, + }, + data() { + const issuableElement = this.$options.el; + const issuableTitleElement = issuableElement.querySelector('.title'); + const issuableDescriptionElement = issuableElement.querySelector('.wiki'); + const issuableDescriptionTextarea = issuableElement.querySelector('.js-task-list-field'); + const { + canUpdate, + endpoint, + issuableRef, + } = issuableElement.dataset; - return new Vue({ - el: issuableElement, - components: { - issuableApp, - }, - render: createElement => createElement('issuable-app', { + return { + canUpdate: gl.utils.convertPermissionToBoolean(canUpdate), + endpoint, + issuableRef, + initialTitle: issuableTitleElement.innerHTML, + initialDescriptionHtml: issuableDescriptionElement ? issuableDescriptionElement.innerHTML : '', + initialDescriptionText: issuableDescriptionTextarea ? issuableDescriptionTextarea.textContent : '', + }; + }, + render(createElement) { + return createElement('issuable-app', { props: { - canUpdate: gl.utils.convertPermissionToBoolean(canUpdate), - endpoint, - issuableRef, - initialTitle: issuableTitleElement.innerHTML, - initialDescriptionHtml: issuableDescriptionElement ? issuableDescriptionElement.innerHTML : '', - initialDescriptionText: issuableDescriptionTextarea ? issuableDescriptionTextarea.textContent : '', + canUpdate: this.canUpdate, + endpoint: this.endpoint, + issuableRef: this.issuableRef, + initialTitle: this.initialTitle, + initialDescriptionHtml: this.initialDescriptionHtml, + initialDescriptionText: this.initialDescriptionText, }, - }), - }); -}); + }); + }, +})); diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index eb73f7cc794..678af978edd 100644 --- a/app/assets/stylesheets/framework/mobile.scss +++ b/app/assets/stylesheets/framework/mobile.scss @@ -112,7 +112,7 @@ } } - .issue_edited_ago, + .issue-edited-ago, .note_edited_ago { display: none; } diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index fbbce6876c2..bc7ff99d483 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -136,11 +136,9 @@ module IssuablesHelper author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "hidden-sm hidden-md hidden-lg") end - if issuable.tasks? - output << " ".html_safe - output << content_tag(:span, issuable.task_status, id: "task_status", class: "hidden-xs hidden-sm") - output << content_tag(:span, issuable.task_status_short, id: "task_status_short", class: "hidden-md hidden-lg") - end + output << " ".html_safe + output << content_tag(:span, issuable.task_status, id: "task_status", class: "hidden-xs hidden-sm") + output << content_tag(:span, issuable.task_status_short, id: "task_status_short", class: "hidden-md hidden-lg") output end diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index bd03593eb98..f66724900de 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -61,7 +61,7 @@ .wiki= markdown_field(@issue, :description) %textarea.hidden.js-task-list-field= @issue.description - = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') + = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue-edited-ago js-issue-edited-ago') #merge-requests{ data: { url: referenced_merge_requests_namespace_project_issue_url(@project.namespace, @project, @issue) } } // This element is filled in using JavaScript. From 3dfce3ab6b092ec40dc95fa292b87f841abf0eba Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 11 May 2017 14:34:56 +0100 Subject: [PATCH 7/7] Fixed karma spec with elements not appearing in DOM --- spec/javascripts/issue_show/components/description_spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js index df9941a2bdb..408349cc42d 100644 --- a/spec/javascripts/issue_show/components/description_spec.js +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -10,6 +10,7 @@ describe('Description component', () => { if (!document.querySelector('.issuable-meta')) { const metaData = document.createElement('div'); metaData.classList.add('issuable-meta'); + metaData.innerHTML = ''; document.body.appendChild(metaData); }