Merge pull request #5757 from nfourre/fix/fix-social-login-button-fill-space

Fill in space in social buttons when options are turned off

app/assets/stylesheets/application.scss

@@ -43,6 +43,7 @@
 @import "budgets/**/*";
 @import "comments/**/*";
 @import "debates/**/*";
+@import "devise/**/*";
 @import "documents/**/*";
 @import "layout/**/*";
 @import "machine_learning/**/*";

app/assets/stylesheets/devise/omniauth_form.scss

@@ -0,0 +1,15 @@
+.devise-omniauth-form {
+  .oauth-logins {
+    display: flex;
+    flex-direction: column;
+
+    @include breakpoint(medium) {
+      flex-direction: row;
+    }
+
+    .oauth-login {
+      @include grid-column-gutter;
+      flex-grow: 1;
+    }
+  }
+}

app/components/devise/omniauth_form_component.html.erb

@@ -1,22 +1,22 @@
-<% if oauth_logins.any? %>
+<div class="row devise-omniauth-form">
-  <div class="row">
+  <div class="small-12 column">
-    <div class="small-12 column">
+    <div class="margin-bottom">
-      <div class="margin-bottom">
+      <strong><%= t("omniauth.info.#{action}") %></strong>
-        <strong><%= t("omniauth.info.#{action}") %></strong>
-      </div>
     </div>
+  </div>
 
+  <div class="oauth-logins small-12">
     <% oauth_logins.each do |login| %>
-      <div class="small-12 medium-6 large-4 column end">
+      <div class="oauth-login">
         <%= link_to t("omniauth.#{login}.name"), send("user_#{login}_omniauth_authorize_path"),
                     title: t("omniauth.#{login}.#{action}"),
                     class: "button-#{login.to_s.delete_suffix("_oauth2")} button expanded",
                     method: :post %>
       </div>
     <% end %>
-
-    <div class="small-12 column auth-divider">
-      <span><%= t("omniauth.or_fill") %></span>
-    </div>
   </div>
-<% end %>
+
+  <div class="small-12 column auth-divider">
+    <span><%= t("omniauth.or_fill") %></span>
+  </div>
+</div>

app/components/devise/omniauth_form_component.rb

@@ -0,0 +1,22 @@
+class Devise::OmniauthFormComponent < ApplicationComponent
+  attr_reader :action
+
+  def initialize(action)
+    @action = action
+  end
+
+  def render?
+    oauth_logins.any?
+  end
+
+  private
+
+    def oauth_logins
+      [
+        (:twitter if feature?(:twitter_login)),
+        (:facebook if feature?(:facebook_login)),
+        (:google_oauth2 if feature?(:google_login)),
+        (:wordpress_oauth2 if feature?(:wordpress_login))
+      ].compact
+    end
+end

app/helpers/settings_helper.rb

@@ -1,13 +1,4 @@
 module SettingsHelper
-  def oauth_logins
-    [
-      (:twitter if feature?(:twitter_login)),
-      (:facebook if feature?(:facebook_login)),
-      (:google_oauth2 if feature?(:google_login)),
-      (:wordpress_oauth2 if feature?(:wordpress_login))
-    ].compact
-  end
-
   def feature?(name)
     setting["feature.#{name}"].presence || setting["process.#{name}"].presence || setting[name].presence
   end

app/views/devise/sessions/new.html.erb

@@ -1,7 +1,7 @@
 <% provide :title, t("devise_views.sessions.new.title") %>
 <h2><%= t("devise_views.sessions.new.title") %></h2>
 
-<%= render "devise/omniauth_form", action: "sign_in" %>
+<%= render Devise::OmniauthFormComponent.new("sign_in") %>
 
 <p>
   <%= sanitize(t("devise_views.shared.links.signup",

app/views/users/registrations/new.html.erb

@@ -1,7 +1,7 @@
 <% provide :title, t("devise_views.users.registrations.new.title") %>
 <h2><%= t("devise_views.users.registrations.new.title") %></h2>
 
-<%= render "devise/omniauth_form", action: "sign_up" %>
+<%= render Devise::OmniauthFormComponent.new("sign_up") %>
 
 <%= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| %>
   <%= render "shared/errors", resource: resource %>

spec/components/devise/omniauth_form_component_spec.rb

@@ -0,0 +1,56 @@
+require "rails_helper"
+
+describe Devise::OmniauthFormComponent do
+  describe "#oauth_logins" do
+    let(:component) { Devise::OmniauthFormComponent.new("sign_up") }
+
+    before do
+      Setting["feature.facebook_login"] = false
+      Setting["feature.twitter_login"] = false
+      Setting["feature.google_login"] = false
+      Setting["feature.wordpress_login"] = false
+    end
+
+    it "is not rendered when all authentications are disabled" do
+      render_inline component
+
+      expect(page).not_to be_rendered
+    end
+
+    it "renders the twitter link when the feature is enabled" do
+      Setting["feature.twitter_login"] = true
+
+      render_inline component
+
+      expect(page).to have_link "Twitter"
+      expect(page).to have_link count: 1
+    end
+
+    it "renders the facebook link when the feature is enabled" do
+      Setting["feature.facebook_login"] = true
+
+      render_inline component
+
+      expect(page).to have_link "Facebook"
+      expect(page).to have_link count: 1
+    end
+
+    it "renders the google link when the feature is enabled" do
+      Setting["feature.google_login"] = true
+
+      render_inline component
+
+      expect(page).to have_link "Google"
+      expect(page).to have_link count: 1
+    end
+
+    it "renders the wordpress link when the feature is enabled" do
+      Setting["feature.wordpress_login"] = true
+
+      render_inline component
+
+      expect(page).to have_link "Wordpress"
+      expect(page).to have_link count: 1
+    end
+  end
+end

spec/system/users_auth_spec.rb

@@ -127,103 +127,6 @@ describe "Users" do
   end
 
   context "OAuth authentication" do
-    context "Form buttons" do
-      before do
-        Setting["feature.facebook_login"] = false
-        Setting["feature.twitter_login"] = false
-        Setting["feature.google_login"] = false
-        Setting["feature.wordpress_login"] = false
-      end
-
-      scenario "No button will appear if all features are disabled" do
-        visit new_user_registration_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-
-        visit new_user_session_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-      end
-
-      scenario "Twitter login button will appear if feature is enabled" do
-        Setting["feature.twitter_login"] = true
-
-        visit new_user_registration_path
-
-        expect(page).to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-
-        visit new_user_session_path
-
-        expect(page).to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-      end
-
-      scenario "Facebook login button will appear if feature is enabled" do
-        Setting["feature.facebook_login"] = true
-
-        visit new_user_registration_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-
-        visit new_user_session_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-      end
-
-      scenario "Google login button will appear if feature is enabled" do
-        Setting["feature.google_login"] = true
-
-        visit new_user_registration_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-
-        visit new_user_session_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).to have_link "Google"
-        expect(page).not_to have_link "Wordpress"
-      end
-
-      scenario "Wordpress login button will appear if feature is enabled" do
-        Setting["feature.wordpress_login"] = true
-
-        visit new_user_registration_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).to have_link "Wordpress"
-
-        visit new_user_session_path
-
-        expect(page).not_to have_link "Twitter"
-        expect(page).not_to have_link "Facebook"
-        expect(page).not_to have_link "Google"
-        expect(page).to have_link "Wordpress"
-      end
-    end
-
     context "Twitter" do
       let(:twitter_hash) { { uid: "12345", info: { name: "manuela" }} }
       let(:twitter_hash_with_email) do