Merge branch 'login-fixups' into 'master'

LDAP login style fixups

## What does this MR do?
* Addresses regression found here: https://gitlab.com/gitlab-org/gitlab-ce/issues/23435
* Adds border radius to left/right tabs

## Are there points in the code the reviewer needs to double check?

So you know what you're seeing, there are three types of fixes going on here: 

1. Template syntax errors that were displaying template-like language in the DOM (specifically with ldap and crowd username labels
2. Aesthetic problems in tabs that arise with more than 2 tabs
3. LDAP and crowd tab functionality. I had to move each panel into its own login box, to ensure that each panel's id synced with its associated tab's href, and that each panel was at an equal depth within the DOM. I assumed bootstrap tabs would look deeper into the login-box to find panels that matched the tabs. This is not true. The reason I didn't catch this is that in development I was just manually changing template flags to force rendering. I didn't realize I could enable ldap in CE within the gitlab.yml file :face_palm:  

See merge request !6947

6 files changed, 32 insertions, 24 deletions

diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss
index 4c0c0839fe2..e6d9be5185d 100644
--- a/app/assets/stylesheets/pages/login.scss
+++ b/app/assets/stylesheets/pages/login.scss
@@ -124,15 +124,20 @@
     display: -webkit-flex;
     display: flex;
     box-shadow: 0 0 0 1px $border-color;
-    border-top-right-radius: 2px;
-    border-top-left-radius: 2px;
+    border-top-right-radius: $border-radius-default;
+    border-top-left-radius: $border-radius-default;
 
     li {
       flex: 1;
       text-align: center;
 
+      &:first-of-type {
+        border-top-left-radius: $border-radius-default;
+      }
+
       &:last-of-type {
         border-left: 1px solid $border-color;
+        border-top-right-radius: $border-radius-default;
       }
 
       &:not(.active) {
diff --git a/app/views/devise/sessions/_new_crowd.html.haml b/app/views/devise/sessions/_new_crowd.html.haml
index e82a08cdb0c..1d381ad7893 100644
--- a/app/views/devise/sessions/_new_crowd.html.haml
+++ b/app/views/devise/sessions/_new_crowd.html.haml
@@ -1,6 +1,6 @@
 = form_tag(omniauth_authorize_path(:user, :crowd), id: 'new_crowd_user', class: 'show-gl-field-errors') do
   .form-group
-    = label_tag 'Username or email', for: :username
+    = label_tag :username, 'Username or email'
     = text_field_tag :username, nil, {class: "form-control top", title: "This field is required", autofocus: "autofocus", required: true }
   .form-group
     = label_tag :password
@@ -10,4 +10,4 @@
       %label{for: "remember_me"}
         = check_box_tag :remember_me, '1', false, id: 'remember_me'
         %span Remember me
-  = button_tag "Sign in", class: "btn-save btn"
+  = submit_tag "Sign in", class: "btn-save btn"
diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml
index b26efbb4535..c18bc2ac413 100644
--- a/app/views/devise/sessions/_new_ldap.html.haml
+++ b/app/views/devise/sessions/_new_ldap.html.haml
@@ -1,6 +1,6 @@
 = form_tag(omniauth_callback_path(:user, server['provider_name']), id: 'new_ldap_user', class: "show-gl-field-errors") do
   .form-group
-    = label_tag "#{server['label']} Login", for: :username
+    = label_tag :username, "#{server['label']} Username"
     = text_field_tag :username, nil, {class: "form-control top", title: "This field is required.", autofocus: "autofocus", required: true }
   .form-group
     = label_tag :password
@@ -10,4 +10,4 @@
       %label{for: "remember_me"}
         = check_box_tag :remember_me, '1', false, id: 'remember_me'
         %span Remember me
-  = button_tag "Sign in", class: "btn-save btn"
+  = submit_tag "Sign in", class: "btn-save btn"
diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml
index 810dd5ab687..86edaf14e43 100644
--- a/app/views/devise/shared/_signin_box.html.haml
+++ b/app/views/devise/shared/_signin_box.html.haml
@@ -1,15 +1,18 @@
-#login-pane.login-box{ role: 'tabpanel', class: 'tab-pane active' }
-  .login-body
-    - if form_based_providers.any?
-      - if crowd_enabled?
-        %div.tab-pane.active{id: "tab-crowd"}
-          = render 'devise/sessions/new_crowd'
-      - @ldap_servers.each_with_index do |server, i|
-        %div.tab-pane{id: "tab-#{server['provider_name']}", class: (:active if i.zero? && !crowd_enabled?)}
-          = render 'devise/sessions/new_ldap', server: server
-      - if signin_enabled?
-        %div#tab-signin.tab-pane
-          = render 'devise/sessions/new_base'
+- if form_based_providers.any?
+  - if crowd_enabled?
+    .login-box.tab-pane.active{id: "crowd", role: 'tabpanel', class: 'tab-pane'}
+      .login-body
+        = render 'devise/sessions/new_crowd'
+  - @ldap_servers.each_with_index do |server, i|
+    .login-box.tab-pane{id: "#{server['provider_name']}", role: 'tabpanel', class: (:active if i.zero? && !crowd_enabled?)}
+      .login-body
+        = render 'devise/sessions/new_ldap', server: server
+  - if signin_enabled?
+    .login-box.tab-pane{id: 'ldap-standard', role: 'tabpanel'}
+      .login-body
+        = render 'devise/sessions/new_base'
 
-    - elsif signin_enabled?
+- elsif signin_enabled?
+  .login-box.tab-pane.active{id: 'login-pane', role: 'tabpanel'}
+    .login-body
       = render 'devise/sessions/new_base'
diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml
index e276e91433a..a057f126c45 100644
--- a/app/views/devise/shared/_tabs_ldap.html.haml
+++ b/app/views/devise/shared/_tabs_ldap.html.haml
@@ -1,10 +1,10 @@
 %ul.new-session-tabs.nav-links.nav-tabs
   - if crowd_enabled?
     %li.active
-      = link_to "Crowd", "#tab-crowd", 'data-toggle' => 'tab'
+      = link_to "Crowd", "#crowd", 'data-toggle' => 'tab'
   - @ldap_servers.each_with_index do |server, i|
     %li{class: (:active if i.zero? && !crowd_enabled?)}
-      = link_to server['label'], "#tab-#{server['provider_name']}", 'data-toggle' => 'tab'
+      = link_to server['label'], "##{server['provider_name']}",  'data-toggle' => 'tab'
   - if signin_enabled?
     %li
-      = link_to 'Standard', '#tab-signin', 'data-toggle' => 'tab'
+      = link_to 'Standard', '#ldap-standard', 'data-toggle' => 'tab'
diff --git a/spec/views/devise/shared/_signin_box.html.haml_spec.rb b/spec/views/devise/shared/_signin_box.html.haml_spec.rb
index ee362e6fcb3..1397bfa5864 100644
--- a/spec/views/devise/shared/_signin_box.html.haml_spec.rb
+++ b/spec/views/devise/shared/_signin_box.html.haml_spec.rb
@@ -12,13 +12,13 @@ describe 'devise/shared/_signin_box' do
 
       render
 
-      expect(rendered).to have_selector('#tab-crowd form')
+      expect(rendered).to have_selector('#crowd form')
     end
 
     it 'is not shown when Crowd is disabled' do
       render
 
-      expect(rendered).not_to have_selector('#tab-crowd')
+      expect(rendered).not_to have_selector('#crowd')
     end
   end