Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/visual_review_toolbar/components/comment.js

 import { BLACK, COMMENT_BOX, MUTED, LOGOUT } from './constants';
-import { clearNote, note, postError } from './note';
-import { buttonClearStyles, selectCommentBox, selectCommentButton, selectNote } from './utils';
+import { clearNote, postError } from './note';
+import {
+  buttonClearStyles,
+  selectCommentBox,
+  selectCommentButton,
+  selectNote,
+  selectNoteContainer,
+} from './utils';
 
 const comment = `
   <div>
     <textarea id="${COMMENT_BOX}" name="${COMMENT_BOX}" rows="3" placeholder="Enter your feedback or idea" class="gitlab-input" aria-required="true"></textarea>
-    ${note}
     <p class="gitlab-metadata-note">Additional metadata will be included: browser, OS, current page, user agent, and viewport dimensions.</p>
   </div>
   <div class="gitlab-button-wrapper">
-    <button class="gitlab-button gitlab-button-secondary" style="${buttonClearStyles}" type="button" id="${LOGOUT}"> Logout </button>
+    <button class="gitlab-button gitlab-button-secondary" style="${buttonClearStyles}" type="button" id="${LOGOUT}"> Log out </button>
     <button class="gitlab-button gitlab-button-success" style="${buttonClearStyles}" type="button" id="gitlab-comment-button"> Send feedback </button>
   </div>
 `;
 
-const resetCommentBox = () => {
-  const commentBox = selectCommentBox();
+const resetCommentButton = () => {
   const commentButton = selectCommentButton();
 
   /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */
   commentButton.innerText = 'Send feedback';
   commentButton.classList.replace('gitlab-button-secondary', 'gitlab-button-success');
   commentButton.style.opacity = 1;
+};
 
+const resetCommentBox = () => {
+  const commentBox = selectCommentBox();
   commentBox.style.pointerEvents = 'auto';
   commentBox.style.color = BLACK;
 };
 
-const resetCommentButton = () => {
+const resetCommentText = () => {
   const commentBox = selectCommentBox();
-  const currentNote = selectNote();
-
   commentBox.value = '';
-  currentNote.innerText = '';
 };
 
 const resetComment = () => {
-  resetCommentBox();
   resetCommentButton();
+  resetCommentBox();
+  resetCommentText();
 };
 
-const confirmAndClear = mergeRequestId => {
+const confirmAndClear = feedbackInfo => {
   const commentButton = selectCommentButton();
   const currentNote = selectNote();
+  const noteContainer = selectNoteContainer();
 
   /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */
   commentButton.innerText = 'Feedback sent';
-  /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */
-  currentNote.innerText = `Your comment was successfully posted to merge request #${mergeRequestId}`;
-  setTimeout(resetComment, 2000);
+  noteContainer.style.visibility = 'visible';
+  currentNote.insertAdjacentHTML('beforeend', feedbackInfo);
+
+  setTimeout(resetComment, 1000);
+  setTimeout(clearNote, 6000);
 };
 
 const setInProgressState = () => {
@@ -71,6 +79,7 @@ const postComment = ({
   innerWidth,
   innerHeight,
   projectId,
+  projectPath,
   mergeRequestId,
   mrUrl,
   token,
@@ -86,6 +95,7 @@ const postComment = ({
     /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */
     postError('Your comment appears to be empty.', COMMENT_BOX);
     resetCommentBox();
+    resetCommentButton();
     return;
   }
 
@@ -114,18 +124,24 @@ const postComment = ({
   })
     .then(response => {
       if (response.ok) {
-        confirmAndClear(mergeRequestId);
-        return;
+        return response.json();
       }
 
       throw new Error(`${response.status}: ${response.statusText}`);
     })
+    .then(data => {
+      const commentId = data.notes[0].id;
+      const feedbackLink = `${mrUrl}/${projectPath}/merge_requests/${mergeRequestId}#note_${commentId}`;
+      const feedbackInfo = `Feedback sent. View at <a class="gitlab-link" href="${feedbackLink}">${projectPath} #${mergeRequestId} (comment ${commentId})</a>`;
+      confirmAndClear(feedbackInfo);
+    })
     .catch(err => {
       postError(
         `Your comment could not be sent. Please try again. Error: ${err.message}`,
         COMMENT_BOX,
       );
       resetCommentBox();
+      resetCommentButton();
     });
 };
 

app/assets/javascripts/visual_review_toolbar/components/constants.js

@@ -2,10 +2,12 @@
 const COLLAPSE_BUTTON = 'gitlab-collapse';
 const COMMENT_BOX = 'gitlab-comment';
 const COMMENT_BUTTON = 'gitlab-comment-button';
-const FORM = 'gitlab-form-wrapper';
+const FORM = 'gitlab-form';
+const FORM_CONTAINER = 'gitlab-form-wrapper';
 const LOGIN = 'gitlab-login';
 const LOGOUT = 'gitlab-logout-button';
 const NOTE = 'gitlab-validation-note';
+const NOTE_CONTAINER = 'gitlab-note-wrapper';
 const REMEMBER_TOKEN = 'gitlab-remember_token';
 const REVIEW_CONTAINER = 'gitlab-review-container';
 const TOKEN_BOX = 'gitlab-token';
@@ -16,16 +18,18 @@ const BLACK = 'rgba(46, 46, 46, 1)';
 const CLEAR = 'rgba(255, 255, 255, 0)';
 const MUTED = 'rgba(223, 223, 223, 0.5)';
 const RED = 'rgba(219, 59, 33, 1)';
-const WHITE = 'rgba(255, 255, 255, 1)';
+const WHITE = 'rgba(250, 250, 250, 1)';
 
 export {
   COLLAPSE_BUTTON,
   COMMENT_BOX,
   COMMENT_BUTTON,
   FORM,
+  FORM_CONTAINER,
   LOGIN,
   LOGOUT,
   NOTE,
+  NOTE_CONTAINER,
   REMEMBER_TOKEN,
   REVIEW_CONTAINER,
   TOKEN_BOX,

app/assets/javascripts/visual_review_toolbar/components/index.js

 import { comment, postComment } from './comment';
-import { COLLAPSE_BUTTON, COMMENT_BUTTON, LOGIN, LOGOUT, REVIEW_CONTAINER } from './constants';
+import {
+  COLLAPSE_BUTTON,
+  COMMENT_BUTTON,
+  FORM_CONTAINER,
+  LOGIN,
+  LOGOUT,
+  REVIEW_CONTAINER,
+} from './constants';
 import { authorizeUser, login } from './login';
+import { note } from './note';
 import { selectContainer } from './utils';
-import { form, logoutUser, toggleForm } from './wrapper';
+import { buttonAndForm, logoutUser, toggleForm } from './wrapper';
 import { collapseButton } from './wrapper_icons';
 
 export {
   authorizeUser,
+  buttonAndForm,
   collapseButton,
   comment,
-  form,
   login,
   logoutUser,
+  note,
   postComment,
   selectContainer,
   toggleForm,
   COLLAPSE_BUTTON,
   COMMENT_BUTTON,
+  FORM_CONTAINER,
   LOGIN,
   LOGOUT,
   REVIEW_CONTAINER,

app/assets/javascripts/visual_review_toolbar/components/login.js

 import { LOGIN, REMEMBER_TOKEN, TOKEN_BOX } from './constants';
-import { clearNote, note, postError } from './note';
+import { clearNote, postError } from './note';
 import { buttonClearStyles, selectRemember, selectToken } from './utils';
 import { addCommentForm } from './wrapper';
 
@@ -7,7 +7,6 @@ const login = `
   <div>
     <label for="${TOKEN_BOX}" class="gitlab-label">Enter your <a class="gitlab-link" href="https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html">personal access token</a></label>
     <input class="gitlab-input" type="password" id="${TOKEN_BOX}" name="${TOKEN_BOX}" aria-required="true" autocomplete="current-password">
-    ${note}
   </div>
   <div class="gitlab-checkbox-wrapper">
     <input type="checkbox" id="${REMEMBER_TOKEN}" name="${REMEMBER_TOKEN}" value="remember">

app/assets/javascripts/visual_review_toolbar/components/note.js

-import { NOTE, RED } from './constants';
-import { selectById, selectNote } from './utils';
+import { NOTE, NOTE_CONTAINER, RED } from './constants';
+import { selectById, selectNote, selectNoteContainer } from './utils';
 
 const note = `
-  <p id=${NOTE} class='gitlab-message'></p>
+  <div id="${NOTE_CONTAINER}" style="visibility: hidden;">
+    <p id="${NOTE}" class="gitlab-message"></p>
+  </div>
 `;
 
 const clearNote = inputId => {
   const currentNote = selectNote();
+  const noteContainer = selectNoteContainer();
+
   currentNote.innerText = '';
   currentNote.style.color = '';
+  noteContainer.style.visibility = 'hidden';
 
   if (inputId) {
     const field = document.getElementById(inputId);
@@ -18,10 +23,13 @@ const clearNote = inputId => {
 
 const postError = (message, inputId) => {
   const currentNote = selectNote();
+  const noteContainer = selectNoteContainer();
   const field = selectById(inputId);
   field.style.borderColor = RED;
   currentNote.style.color = RED;
   currentNote.innerText = message;
+  noteContainer.style.visibility = 'visible';
+  setTimeout(clearNote.bind(null, inputId), 5000);
 };
 
 export { clearNote, note, postError };

app/assets/javascripts/visual_review_toolbar/components/utils.js

@@ -5,7 +5,9 @@ import {
   COMMENT_BOX,
   COMMENT_BUTTON,
   FORM,
+  FORM_CONTAINER,
   NOTE,
+  NOTE_CONTAINER,
   REMEMBER_TOKEN,
   REVIEW_CONTAINER,
   TOKEN_BOX,
@@ -24,7 +26,9 @@ const selectCommentBox = () => document.getElementById(COMMENT_BOX);
 const selectCommentButton = () => document.getElementById(COMMENT_BUTTON);
 const selectContainer = () => document.getElementById(REVIEW_CONTAINER);
 const selectForm = () => document.getElementById(FORM);
+const selectFormContainer = () => document.getElementById(FORM_CONTAINER);
 const selectNote = () => document.getElementById(NOTE);
+const selectNoteContainer = () => document.getElementById(NOTE_CONTAINER);
 const selectRemember = () => document.getElementById(REMEMBER_TOKEN);
 const selectToken = () => document.getElementById(TOKEN_BOX);
 
@@ -36,7 +40,9 @@ export {
   selectCommentBox,
   selectCommentButton,
   selectForm,
+  selectFormContainer,
   selectNote,
+  selectNoteContainer,
   selectRemember,
   selectToken,
 };

app/assets/javascripts/visual_review_toolbar/components/wrapper.js

 import { comment } from './comment';
-import { CLEAR, FORM, WHITE } from './constants';
+import { CLEAR, FORM, FORM_CONTAINER, WHITE } from './constants';
 import { login } from './login';
-import { selectCollapseButton, selectContainer, selectForm } from './utils';
+import { clearNote } from './note';
+import {
+  selectCollapseButton,
+  selectForm,
+  selectFormContainer,
+  selectNoteContainer,
+} from './utils';
 import { commentIcon, compressIcon } from './wrapper_icons';
 
 const form = content => `
-  <form id=${FORM}>
+  <form id="${FORM}">
     ${content}
   </form>
 `;
 
+const buttonAndForm = ({ content, toggleButton }) => `
+  <div id="${FORM_CONTAINER}" class="gitlab-form-open">
+    ${toggleButton}
+    ${form(content)}
+  </div>
+`;
+
 const addCommentForm = () => {
   const formWrapper = selectForm();
   formWrapper.innerHTML = comment;
@@ -31,13 +44,15 @@ function logoutUser() {
     return;
   }
 
+  clearNote();
   addLoginForm();
 }
 
 function toggleForm() {
-  const container = selectContainer();
   const collapseButton = selectCollapseButton();
   const currentForm = selectForm();
+  const formContainer = selectFormContainer();
+  const noteContainer = selectNoteContainer();
   const OPEN = 'open';
   const CLOSED = 'closed';
 
@@ -49,7 +64,7 @@ function toggleForm() {
 
   const openButtonClasses = ['gitlab-collapse-closed', 'gitlab-collapse-open'];
   const closedButtonClasses = [...openButtonClasses].reverse();
-  const openContainerClasses = ['gitlab-closed-wrapper', 'gitlab-open-wrapper'];
+  const openContainerClasses = ['gitlab-wrapper-closed', 'gitlab-wrapper-open'];
   const closedContainerClasses = [...openContainerClasses].reverse();
 
   const stateVals = {
@@ -72,11 +87,16 @@ function toggleForm() {
   const nextState = collapseButton.classList.contains('gitlab-collapse-open') ? CLOSED : OPEN;
   const currentVals = stateVals[nextState];
 
-  container.classList.replace(...currentVals.containerClasses);
-  container.style.backgroundColor = currentVals.backgroundColor;
+  formContainer.classList.replace(...currentVals.containerClasses);
+  formContainer.style.backgroundColor = currentVals.backgroundColor;
+  formContainer.classList.toggle('gitlab-form-open');
   currentForm.style.display = currentVals.display;
   collapseButton.classList.replace(...currentVals.buttonClasses);
   collapseButton.innerHTML = currentVals.icon;
+
+  if (noteContainer && noteContainer.innerText.length > 0) {
+    noteContainer.style.display = currentVals.display;
+  }
 }
 
-export { addCommentForm, addLoginForm, form, logoutUser, toggleForm };
+export { addCommentForm, addLoginForm, buttonAndForm, logoutUser, toggleForm };

app/assets/javascripts/visual_review_toolbar/index.js

 import './styles/toolbar.css';
 
-import { form, selectContainer, REVIEW_CONTAINER } from './components';
+import { buttonAndForm, note, selectContainer, REVIEW_CONTAINER } from './components';
 import { debounce, eventLookup, getInitialView, initializeState, updateWindowSize } from './store';
 
 /*
@@ -20,12 +20,11 @@ import { debounce, eventLookup, getInitialView, initializeState, updateWindowSiz
 window.addEventListener('load', () => {
   initializeState(window, document);
 
-  const { content, toggleButton } = getInitialView(window);
+  const mainContent = buttonAndForm(getInitialView(window));
   const container = document.createElement('div');
-
   container.setAttribute('id', REVIEW_CONTAINER);
-  container.insertAdjacentHTML('beforeend', toggleButton);
-  container.insertAdjacentHTML('beforeend', form(content));
+  container.insertAdjacentHTML('beforeend', note);
+  container.insertAdjacentHTML('beforeend', mainContent);
 
   document.body.insertBefore(container, document.body.firstChild);
 

app/assets/javascripts/visual_review_toolbar/store/state.js

@@ -34,7 +34,7 @@ const initializeState = (wind, doc) => {
   const browser = getBrowserId(userAgent);
 
   const scriptEl = doc.getElementById('review-app-toolbar-script');
-  const { projectId, mergeRequestId, mrUrl } = scriptEl.dataset;
+  const { projectId, mergeRequestId, mrUrl, projectPath } = scriptEl.dataset;
 
   // This mutates our default state object above. It's weird but it makes the linter happy.
   Object.assign(state, {
@@ -46,6 +46,7 @@ const initializeState = (wind, doc) => {
     mrUrl,
     platform,
     projectId,
+    projectPath,
     userAgent,
   });
 };

app/assets/javascripts/visual_review_toolbar/styles/toolbar.css

@@ -6,23 +6,42 @@
   pointer-events: none;
 }
 
-#gitlab-form-wrapper {
+#gitlab-comment {
+  background-color: #fafafa;
+}
+
+#gitlab-form {
+  display: flex;
+  flex-direction: column;
+  width: 100%;
+  margin-bottom: 0;
+}
+
+#gitlab-note-wrapper {
   display: flex;
   flex-direction: column;
-  width: 100%
+  background-color: #fafafa;
+  border-radius: 4px;
+  margin-bottom: .5rem;
+  padding: 1rem;
+}
+
+#gitlab-form-wrapper {
+  overflow: auto;
+  display: flex;
+  flex-direction: row-reverse;
+  border-radius: 4px;
 }
 
 #gitlab-review-container {
   max-width: 22rem;
   max-height: 22rem;
-  overflow: scroll;
+  overflow: auto;
+  display: flex;
+  flex-direction: column;
   position: fixed;
   bottom: 1rem;
   right: 1rem;
-  display: flex;
-  flex-direction: row-reverse;
-  padding: 1rem;
-  background-color: #fff;
   font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen-Sans, Ubuntu, Cantarell,
   'Helvetica Neue', sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol',
   'Noto Color Emoji';
@@ -31,12 +50,12 @@
   color: #2e2e2e;
 }
 
-.gitlab-open-wrapper {
+.gitlab-wrapper-open {
   max-width: 22rem;
   max-height: 22rem;
 }
 
-.gitlab-closed-wrapper {
+.gitlab-wrapper-closed {
   max-width: 3.4rem;
   max-height: 3.4rem;
 }
@@ -47,7 +66,7 @@
 }
 
 .gitlab-button-secondary {
-  background: none #fff;
+  background: none #fafafa;
   margin: 0 .5rem;
   border: 1px solid #e3e3e3;
 }
@@ -113,6 +132,11 @@
   align-items: baseline;
 }
 
+.gitlab-form-open {
+  padding: 1rem;
+  background-color: #fafafa;
+}
+
 .gitlab-label {
   font-weight: 600;
   display: inline-block;
@@ -126,6 +150,10 @@
   background-image: none;
 }
 
+.gitlab-link:hover {
+  text-decoration: underline;
+}
+
 .gitlab-message {
   padding: .25rem 0;
   margin: 0;

changelogs/unreleased/63247-add-conf-toast-and-link.yml

+---
+title: Include a link back to the MR for Visual Review feedback form
+merge_request: 29719
+author:
+type: changed

spec/features/groups/issues_spec.rb

@@ -102,14 +102,16 @@ describe 'Group issues page' do
   end
 
   context 'manual ordering' do
+    let(:user_in_group) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
+
+    let!(:issue1) { create(:issue, project: project, title: 'Issue #1', relative_position: 1) }
+    let!(:issue2) { create(:issue, project: project, title: 'Issue #2', relative_position: 2) }
+    let!(:issue3) { create(:issue, project: project, title: 'Issue #3', relative_position: 3) }
+
     before do
-      stub_feature_flags(manual_sorting: true)
+      sign_in(user_in_group)
     end
 
-    let!(:issue1) { create(:issue, project: project, title: 'Issue #1') }
-    let!(:issue2) { create(:issue, project: project, title: 'Issue #2') }
-    let!(:issue3) { create(:issue, project: project, title: 'Issue #3') }
-
     it 'displays all issues' do
       visit issues_group_path(group, sort: 'relative_position')
 
@@ -136,14 +138,19 @@ describe 'Group issues page' do
       visit issues_group_path(group, sort: 'relative_position')
 
       drag_to(selector: '.manual-ordering',
-        scrollable: '#board-app',
-        list_from_index: 0,
         from_index: 0,
-        to_index: 2,
-        list_to_index: 0)
+        to_index: 2)
 
       wait_for_requests
 
+      check_issue_order
+
+      visit issues_group_path(group, sort: 'relative_position')
+
+      check_issue_order
+    end
+
+    def check_issue_order
       page.within('.manual-ordering') do
         expect(find('.issue:nth-child(1) .title')).to have_content('Issue #2')
         expect(find('.issue:nth-child(2) .title')).to have_content('Issue #3')