From 18e2388de19f47093cb3192f4b8dbabdd9c3bfad Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Fri, 7 Apr 2017 14:09:15 +0100 Subject: [PATCH] Fixed issue button state bug --- app/assets/javascripts/comment_type_toggle.js | 67 +++++++----- app/assets/javascripts/notes.js | 31 ++++-- app/assets/stylesheets/pages/note_form.scss | 1 - .../projects/notes/_comment_button.html.haml | 5 +- spec/javascripts/comment_type_toggle_spec.js | 101 +++++++++++------- 5 files changed, 123 insertions(+), 82 deletions(-) diff --git a/app/assets/javascripts/comment_type_toggle.js b/app/assets/javascripts/comment_type_toggle.js index 34878f90050..ba3b43b201d 100644 --- a/app/assets/javascripts/comment_type_toggle.js +++ b/app/assets/javascripts/comment_type_toggle.js @@ -2,39 +2,54 @@ import DropLab from '~/droplab/drop_lab'; import InputSetter from '~/droplab/plugins/input_setter'; class CommentTypeToggle { - constructor(dropdownTrigger, dropdownList, noteTypeInput, submitButton, closeButton) { - this.dropdownTrigger = dropdownTrigger; - this.dropdownList = dropdownList; - this.noteTypeInput = noteTypeInput; - this.submitButton = submitButton; - this.closeButton = closeButton; + constructor(opts = {}) { + this.dropdownTrigger = opts.dropdownTrigger; + this.dropdownList = opts.dropdownList; + this.noteTypeInput = opts.noteTypeInput; + this.submitButton = opts.submitButton; + this.closeButton = opts.closeButton; + this.reopenButton = opts.reopenButton; } initDroplab() { this.droplab = new DropLab(); - const inputSetterConfig = [{ - input: this.noteTypeInput, - valueAttribute: 'data-value', - }, - { - input: this.submitButton, - valueAttribute: 'data-button-text', - }]; - if (this.closeButton) { - inputSetterConfig.push({ - input: this.closeButton, - valueAttribute: 'data-secondary-button-text', - }, { - input: this.closeButton, - valueAttribute: 'data-secondary-button-text', - inputAttribute: 'data-alternative-text', - }); - } + const config = this.setConfig(); - this.droplab.init(this.dropdownTrigger, this.dropdownList, [InputSetter], { - InputSetter: inputSetterConfig, + this.droplab.init(this.dropdownTrigger, this.dropdownList, [InputSetter], config); + } + + setConfig() { + const config = { + InputSetter: [{ + input: this.noteTypeInput, + valueAttribute: 'data-value', + }, + { + input: this.submitButton, + valueAttribute: 'data-submit-text', + }], + }; + + if (!this.closeButton || !this.reopenButton) return config; + + config.InputSetter.push({ + input: this.closeButton, + valueAttribute: 'data-close-text', + }, { + input: this.closeButton, + valueAttribute: 'data-close-text', + inputAttribute: 'data-alternative-text', + }, { + input: this.reopenButton, + valueAttribute: 'data-reopen-text', + }, { + input: this.reopenButton, + valueAttribute: 'data-reopen-text', + inputAttribute: 'data-alternative-text', }); + + return config; } } diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9dd516d93fd..2a8f4acc72a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -137,16 +137,24 @@ require('./task_list'); $(document).off("click", '.system-note-commit-list-toggler'); }; - Notes.prototype.initCommentTypeToggle = function (form) { - this.commentTypeToggle = new CommentTypeToggle( - form.querySelector('.js-comment-type-dropdown .dropdown-toggle'), - form.querySelector('.js-comment-type-dropdown .dropdown-menu'), - form.querySelector('#note_type'), - form.querySelector('.js-comment-type-dropdown .js-comment-submit-button'), - form.querySelector('.js-note-target-close:not(.hidden)') || form.querySelector('.js-note-target-reopen'), - ); + Notes.initCommentTypeToggle = function (form) { + const dropdownTrigger = form.querySelector('.js-comment-type-dropdown .dropdown-toggle'); + const dropdownList = form.querySelector('.js-comment-type-dropdown .dropdown-menu'); + const noteTypeInput = form.querySelector('#note_type'); + const submitButton = form.querySelector('.js-comment-type-dropdown .js-comment-submit-button'); + const closeButton = form.querySelector('.js-note-target-close'); + const reopenButton = form.querySelector('.js-note-target-reopen'); - this.commentTypeToggle.initDroplab(); + const commentTypeToggle = new CommentTypeToggle({ + dropdownTrigger, + dropdownList, + noteTypeInput, + submitButton, + closeButton, + reopenButton, + }); + + commentTypeToggle.initDroplab(); }; Notes.prototype.keydownNoteText = function(e) { @@ -470,7 +478,7 @@ require('./task_list'); this.parentTimeline = form.parents('.timeline'); if (form.length) { - this.initCommentTypeToggle(form.get(0)); + Notes.initCommentTypeToggle(form.get(0)); } }; @@ -939,8 +947,9 @@ require('./task_list'); reopenbtn = form.find('.js-note-target-reopen'); closebtn = form.find('.js-note-target-close'); discardbtn = form.find('.js-note-discard'); + if (textarea.val().trim().length > 0) { - reopentext = reopenbtn.data('alternative-text'); + reopentext = reopenbtn.attr('data-alternative-text'); closetext = closebtn.attr('data-alternative-text'); if (reopenbtn.text() !== reopentext) { reopenbtn.text(reopentext); diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 09c178c5a14..c71a4e717f8 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -334,7 +334,6 @@ li { white-space: nowrap; - border-radius: 0; cursor: pointer; padding-top: 6px; diff --git a/app/views/projects/notes/_comment_button.html.haml b/app/views/projects/notes/_comment_button.html.haml index 2e65a3bf294..8627713fd68 100644 --- a/app/views/projects/notes/_comment_button.html.haml +++ b/app/views/projects/notes/_comment_button.html.haml @@ -1,5 +1,4 @@ - noteable_name = @note.noteable.human_class_name -- noteable_state_action = noteable_name =~ /(merge request|issue)/ && @note.noteable['state'] == 'closed' ? 'reopen' : 'close' .pull-left.btn-group.append-right-10.comment-type-dropdown.js-comment-type-dropdown %input.btn.btn-nr.btn-create.comment-btn.js-comment-button.js-comment-submit-button{ type: 'submit', value: 'Comment' } @@ -9,7 +8,7 @@ = icon('caret-down') %ul#resolvable-comment-menu.dropdown-menu{ data: { dropdown: true } } - %li#comment.droplab-item-selected{ data: { value: '', 'button-text' => 'Comment', 'secondary-button-text' => "Comment & #{noteable_state_action} #{noteable_name}" } } + %li#comment.droplab-item-selected{ data: { value: '', 'submit-text' => 'Comment', 'close-text' => "Comment & close #{noteable_name}", 'reopen-text' => "Comment & reopen #{noteable_name}" } } = icon('check') .description %strong Comment @@ -18,7 +17,7 @@ %li.divider - %li#discussion{ data: { value: 'DiscussionNote', 'button-text' => 'Start discussion', 'secondary-button-text' => "Start discussion & #{noteable_state_action} #{noteable_name}" } } + %li#discussion{ data: { value: 'DiscussionNote', 'submit-text' => 'Start discussion', 'close-text' => "Start discussion & close #{noteable_name}", 'reopen-text' => "Start discussion & reopen #{noteable_name}" } } = icon('check') .description %strong Start discussion diff --git a/spec/javascripts/comment_type_toggle_spec.js b/spec/javascripts/comment_type_toggle_spec.js index 818c1635710..9a341ebd811 100644 --- a/spec/javascripts/comment_type_toggle_spec.js +++ b/spec/javascripts/comment_type_toggle_spec.js @@ -11,13 +11,13 @@ describe('CommentTypeToggle', function () { this.submitButton = {}; this.closeButton = {}; - this.commentTypeToggle = new CommentTypeToggle( - this.dropdownTrigger, - this.dropdownList, - this.noteTypeInput, - this.submitButton, - this.closeButton, - ); + this.commentTypeToggle = new CommentTypeToggle({ + dropdownTrigger: this.dropdownTrigger, + dropdownList: this.dropdownList, + noteTypeInput: this.noteTypeInput, + submitButton: this.submitButton, + closeButton: this.closeButton, + }); }); it('should set .dropdownTrigger', function () { @@ -39,6 +39,10 @@ describe('CommentTypeToggle', function () { it('should set .closeButton', function () { expect(this.commentTypeToggle.closeButton).toBe(this.closeButton); }); + + it('should set .reopenButton', function () { + expect(this.commentTypeToggle.reopenButton).toBe(this.reopenButton); + }); }); describe('initDroplab', function () { @@ -49,13 +53,16 @@ describe('CommentTypeToggle', function () { noteTypeInput: {}, submitButton: {}, closeButton: {}, + setConfig: () => {}, }; + this.config = {}; this.droplab = jasmine.createSpyObj('droplab', ['init']); spyOn(dropLabSrc, 'default').and.returnValue(this.droplab); + spyOn(this.commentTypeToggle, 'setConfig').and.returnValue(this.config); - this.initDroplab = CommentTypeToggle.prototype.initDroplab.call(this.commentTypeToggle); + CommentTypeToggle.prototype.initDroplab.call(this.commentTypeToggle); }); it('should instantiate a DropLab instance', function () { @@ -66,31 +73,21 @@ describe('CommentTypeToggle', function () { expect(this.commentTypeToggle.droplab).toBe(this.droplab); }); + it('should call .setConfig', function () { + expect(this.commentTypeToggle.setConfig).toHaveBeenCalled(); + }); + it('should call DropLab.prototype.init', function () { expect(this.droplab.init).toHaveBeenCalledWith( this.commentTypeToggle.dropdownTrigger, this.commentTypeToggle.dropdownList, [InputSetter], - { - InputSetter: [{ - input: this.commentTypeToggle.noteTypeInput, - valueAttribute: 'data-value', - }, { - input: this.commentTypeToggle.submitButton, - valueAttribute: 'data-button-text', - }, - { - input: this.commentTypeToggle.closeButton, - valueAttribute: 'data-secondary-button-text', - }, { - input: this.commentTypeToggle.closeButton, - valueAttribute: 'data-secondary-button-text', - inputAttribute: 'data-alternative-text', - }], - }, + this.config, ); }); + }); + describe('setConfig', function () { describe('if no .closeButton is provided', function () { beforeEach(function () { this.commentTypeToggle = { @@ -98,26 +95,48 @@ describe('CommentTypeToggle', function () { dropdownList: {}, noteTypeInput: {}, submitButton: {}, + reopenButton: {}, }; - this.initDroplab = CommentTypeToggle.prototype.initDroplab.call(this.commentTypeToggle); + this.setConfig = CommentTypeToggle.prototype.setConfig.call(this.commentTypeToggle); }); - it('should not add .closeButton related InputSetter config', function () { - expect(this.droplab.init).toHaveBeenCalledWith( - this.commentTypeToggle.dropdownTrigger, - this.commentTypeToggle.dropdownList, - [InputSetter], - { - InputSetter: [{ - input: this.commentTypeToggle.noteTypeInput, - valueAttribute: 'data-value', - }, { - input: this.commentTypeToggle.submitButton, - valueAttribute: 'data-button-text', - }], - }, - ); + it('should not add .closeButton or .reopenButton related InputSetter config', function () { + expect(this.setConfig).toEqual({ + InputSetter: [{ + input: this.commentTypeToggle.noteTypeInput, + valueAttribute: 'data-value', + }, { + input: this.commentTypeToggle.submitButton, + valueAttribute: 'data-submit-text', + }], + }); + }); + }); + + describe('if no .reopenButton is provided', function () { + beforeEach(function () { + this.commentTypeToggle = { + dropdownTrigger: {}, + dropdownList: {}, + noteTypeInput: {}, + submitButton: {}, + closeButton: {}, + }; + + this.setConfig = CommentTypeToggle.prototype.setConfig.call(this.commentTypeToggle); + }); + + it('should not add .closeButton or .reopenButton related InputSetter config', function () { + expect(this.setConfig).toEqual({ + InputSetter: [{ + input: this.commentTypeToggle.noteTypeInput, + valueAttribute: 'data-value', + }, { + input: this.commentTypeToggle.submitButton, + valueAttribute: 'data-submit-text', + }], + }); }); }); });