Skip to content

Allow Merge Request comment type to be selected without text in the box

We've been playing with the feature added to resolve gitlab-ce#24378 in 9.1, and I noticed that I can't switch between "Comment" and "Start discussion" until I start typing. This breaks my natural flow a little bit, since I would like to set the type before I write anything to ensure that I don't accidentally create the wrong thing.

While gitlab-ce#29078 would alleviate this a little bit, it's not a general solution.

Proposal

Don't disable arrow portion of dropdown in order to switch comment type. Keep comment button disabled until text is entered to prevent submitting empty comments.

cisPqz22iY

Here are the desirable states of the buttons for this proposal where the dropdown is always enabled.

  1. Comment selected

    1. Without input: Comment disabled, Close issue enabled.

    2. With input: Comment enabled, Comment & close merge request enabled.

  2. Start discussion selected

    1. Without input: Start discussion disabled, Start discussion & close merge request disabled.

    2. With input: Start discussion enabled, Start discussion & close merge request enabled.

The same set of states work the same for the reopen case and also the same close and reopen cases for issues.

This can almost be solved here: app/assets/javascripts/gl_form.js#L34

Original code… The dropdown toggle button is .js-note-new-discussion.

gl.utils.disableButtonIfEmptyField(this.form.find('.js-note-text'),
    this.form.find('.js-comment-button, .js-note-new-discussion'));

Removing .js-note-new-discussion accomplishes 1.1, 1.2, & 2.2. But not 2.1. In this case the Start discussion & close merge request remains enabled with no input.

gl.utils.disableButtonIfEmptyField(this.form.find('.js-note-text'),
    this.form.find('.js-comment-button'));

Replacing .js-note-new-discussion with .js-note-target-close accomplishes 1.2, 2.1, & 2.2. But not 1.1. In this case, the Close merge request button is disabled.

gl.utils.disableButtonIfEmptyField(this.form.find('.js-note-text'),
    this.form.find('.js-comment-button, .js-note-target-close'));
Relevant files

  • app/assets/javascripts/comment_type_toggle.js

  • app/assets/javascripts/gl_form.js

  • app/assets/javascripts/notes.js

  • app/views/projects/merge_requests/_discussion.html.haml

  • app/views/shared/notes/_comment_button.html.haml

  • app/views/shared/notes/_form.html.haml

Edited by Taurie Davis