Commit 24b73bc9 authored by Rémy Coutable's avatar Rémy Coutable

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 🤦  

See merge request !6947
parents ec7b6b9f 9a6f73a6
......@@ -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) {
......
= 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"
= 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"
#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'
%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'
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment