[OP-17129] Improve and modernise the login screen / dropdown

app/forms/login_form.rb

@@ -55,37 +55,28 @@ class LoginForm < ApplicationForm
       autocomplete: "current-password"
     )
 
-    if Setting::Autologin.enabled?
-      f.check_box name: "autologin",
-                  id: "autologin#{@id_suffix}",
-                  checked: false,
-                  value: 1,
-                  label: I18n.t("users.autologins.prompt",
-                                num_days: I18n.t("datetime.distance_in_words.x_days", count: Setting.autologin))
-    end
-
-    f.html_content do
-      flex_layout(justify_content: :space_between, align_items: :center) do |flex|
-        flex.with_column do
-          render(Primer::Beta::Button.new(type: :submit, scheme: :primary)) { I18n.t(:button_login) }
-        end
+    f.group(layout: :horizontal,
+            justify_content: Setting::Autologin.enabled? ? :space_between : :flex_end,
+            align_items: :center) do |g|
+      if Setting::Autologin.enabled?
+        g.check_box name: "autologin",
+                    id: "autologin#{@id_suffix}",
+                    checked: false,
+                    value: 1,
+                    label: I18n.t("users.autologins.prompt",
+                                  num_days: I18n.t("datetime.distance_in_words.x_days", count: Setting.autologin))
+      end
 
-        flex.with_column do
-          flex_layout do |links|
-            if Setting::SelfRegistration.enabled?
-              links.with_row do
-                render(Primer::Beta::Link.new(href: url_helpers.account_register_path)) { I18n.t(:label_register) }
-              end
-            end
-            if Setting.lost_password?
-              links.with_row do
-                render(Primer::Beta::Link.new(href: url_helpers.account_lost_password_path)) { I18n.t(:label_password_lost) }
-              end
-            end
-          end
+      if Setting.lost_password?
+        g.html_content do
+          render(Primer::Beta::Link.new(href: url_helpers.account_lost_password_path)) { I18n.t(:label_password_lost) }
         end
       end
     end
+
+    f.html_content do
+      render(Primer::Beta::Button.new(type: :submit, scheme: :primary, block: true)) { I18n.t(:button_login) }
+    end
   end
 
   def initialize(back_url: nil, username: nil, id_suffix: nil)

app/forms/registration_form.rb

@@ -55,7 +55,9 @@ class RegistrationForm < ApplicationForm
 
     consent(f) if helpers.user_consent_required?
 
-    f.submit(name: :submit, label: I18n.t(:button_create), scheme: :primary)
+    f.html_content do
+      render(Primer::Beta::Button.new(type: :submit, scheme: :primary, block: true)) { I18n.t(:button_create) }
+    end
 
     registration_footer(f)
   end

app/forms/settings/input_methods.rb

@@ -203,27 +203,30 @@ def select_list(name:, values: nil, option_options: {}, **, &block)
     end
 
     def multi_language_text_select(name:, current_language: I18n.locale.to_s)
-      # Add select list to switch
-      object.select_list(
-        name: :"#{name}_lang", # Should be excluded by settings params
-        input_width: :small,
-        id: "lang-for-#{name}",
-        class: "lang-select-switch",
-        label: setting_label(name),
-        caption: setting_caption(name),
-        include_blank: false
-      ) do |select|
-        lang_options_for_select(false).each do |label, value|
-          select.option(
-            value:,
-            label:,
-            selected: value == current_language
-          )
+      object.group(data: { controller: "admin--multi-lang-text-setting" }) do |group|
+        # Add select list to switch
+        group.select_list(
+          name: :"#{name}_lang", # Should be excluded by settings params
+          input_width: :small,
+          id: "lang-for-#{name}",
+          class: "lang-select-switch",
+          label: setting_label(name),
+          caption: setting_caption(name),
+          include_blank: false,
+          data: { admin__multi_lang_text_setting_target: "select" }
+        ) do |select|
+          lang_options_for_select(false).each do |label, value|
+            select.option(
+              value:,
+              label:,
+              selected: value == current_language
+            )
+          end
         end
-      end
 
-      object.fields_for(name) do |builder|
-        MultiLangForm.new(builder, name:, current_language:)
+        group.fields_for(name) do |builder|
+          MultiLangForm.new(builder, name:, current_language:)
+        end
       end
     end
 

app/forms/settings/multi_lang_form.rb

@@ -47,7 +47,8 @@ def initialize(name:, current_language:)
         f.hidden(
           name: lang,
           value: Setting.send(name)[lang],
-          id: "lang-for-#{name}-#{lang}"
+          id: "lang-for-#{name}-#{lang}",
+          data: { admin__multi_lang_text_setting_target: "langFor", lang: }
         )
       end
 
@@ -58,6 +59,7 @@ def initialize(name:, current_language:)
         label: setting_label(name),
         disabled: setting_disabled?(name),
         visually_hide_label: true,
+        data: { admin__multi_lang_text_setting_target: "textArea" },
         rich_text_options: {
           turboMode: true,
           showAttachments: false

app/helpers/omniauth_helper.rb

@@ -43,16 +43,17 @@ def omniauth_login_path_options
     end
   end
 
-  def omniauth_provider_button(name, display_name: nil, icon: false)
-    classes = ["auth-provider", "auth-provider-#{name}", "button"]
-    classes << "auth-provider--imaged" if icon
+  def omniauth_provider_button(name, display_name: nil, icon: nil)
+    render(Primer::Beta::Button.new(tag: :a,
+                                    href: omniauth_login_path(name, omniauth_login_path_options),
+                                    block: true,
+                                    align_content: :center,
+                                    classes: "auth-provider auth-provider-#{name}",
+                                    data: { turbo: false })) do
+      label = tag.span(provider_button_label(name, display_name), class: "auth-provider-name")
+      parts = icon.present? ? [image_tag(icon, alt: "", class: "auth-provider-icon"), label] : [label]
 
-    link_to(
-      omniauth_login_path(name, omniauth_login_path_options),
-      class: classes.join(" "),
-      data: { turbo: false }
-    ) do
-      tag.span(display_name.presence || name, class: "auth-provider-name")
+      render(Primer::Alpha::Stack.new(direction: :horizontal, align: :center, gap: :condensed)) { safe_join(parts) }
     end
   end
 
@@ -70,6 +71,20 @@ def direct_login_provider
     Setting.omniauth_direct_login_provider.presence
   end
 
+  def provider_button_label(name, display_name)
+    I18n.t("account.continue_with_provider", name: display_name.presence || name)
+  end
+
+  # Returns nil unless the hook produced actual provider markup. In development
+  # Rails annotates every rendered partial with HTML comments, so a hook that
+  # rendered nothing still comes back non-blank.
+  def auth_provider_html
+    html = call_hook :view_account_login_auth_provider
+    content = Rails.env.development? ? html.gsub(/<!--.*?-->/m, "") : html
+
+    html if content.present?
+  end
+
   # Uses the controller-assigned flag so /login/internal can still render the
   # password form when instance-wide password login is off.
   def show_password_login_form?

app/views/account/_auth_providers.html.erb

@@ -35,22 +35,28 @@ See COPYRIGHT and LICENSE files for more details.
 # See
 # * https://community.openproject.org/work_packages/7192
 # * http://stackoverflow.com/questions/13112430/find-loaded-providers-for-omniauth
-auth_provider_html = call_hook :view_account_login_auth_provider
+providers_html = auth_provider_html
 no_pwd = Users::PasswordLogin.none? && !@force_password_login_form
-pclass = no_pwd ? "no-pwd" : ""
-wclass = local_assigns[:wide] ? "wide" : ""
+# Without a password form above there is nothing to separate the section from.
+spacing = no_pwd ? { mt: 0 } : { mt: 4, pt: 4, border: :top }
+title = Primer::Beta::Heading.new(
+  tag: :h3,
+  classes: "login-auth-providers-title",
+  font_size: 4,
+  font_weight: :bold,
+  text_align: :center,
+  mb: 3
+)
 %>
-<% if auth_provider_html.present? %>
-  <div class="login-auth-providers <%= pclass %> <%= wclass %>">
+<% if providers_html %>
+  <%= render(Primer::Box.new(classes: "login-auth-providers", **spacing)) do %>
     <% unless no_pwd %>
-      <h3 class="login-auth-providers-title mb-3">
-        <span>
-          <%= local_assigns[:omniauth_title] || I18n.t("account.login_with_auth_provider") %>
-        </span>
-      </h3>
+      <%= render(title) do %>
+        <%= local_assigns[:omniauth_title] || I18n.t("account.login_with_external_account") %>
+      <% end %>
     <% end %>
-    <div class="login-auth-provider-list">
-      <%= auth_provider_html %>
-    </div>
-  </div>
+    <%= render(Primer::Alpha::Stack.new(classes: "login-auth-provider-list", direction: :vertical, gap: :condensed)) do %>
+      <%= providers_html %>
+    <% end %>
+  <% end %>
 <% end %>

app/views/account/_login.html.erb

@@ -45,4 +45,7 @@ See COPYRIGHT and LICENSE files for more details.
   %>
 
   <%= render partial: "account/auth_providers" %>
+  <% if Setting::SelfRegistration.enabled? %>
+    <%= render partial: "account/signup_prompt" %>
+  <% end %>
 </div>

modules/auth_plugins/app/views/hooks/login/_providers_css.html.erb → app/views/account/_signin_prompt.html.erb

@@ -21,24 +21,16 @@ GNU General Public License for more details.
 
 You should have received a copy of the GNU General Public License
 along with this program; if not, write to the Free Software
-Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
 See COPYRIGHT and LICENSE files for more details.
 
 ++#%>
-
-<% OpenProject::Plugins::AuthPlugin.providers.each do |provider| %>
-  <% if provider[:icon] %>
-    <style>
-      #content .login-auth-providers a.auth-provider.auth-provider-<%= provider[:name] %> {
-        background-image: url('<%= asset_path(provider[:icon]) %>');
-      }
-      .op-app-header #nav-login-content .login-auth-providers a.auth-provider.auth-provider-<%= provider[:name] %> {
-        background-image:  url('<%= asset_path(provider[:icon]) %>') ;
-      }
-      .login-auth-providers a.auth-provider.auth-provider-<%= provider[:name] %> {
-        background-image: url('<%= asset_path(provider[:icon]) %>') ;
-      }
-    </style>
-  <% end -%>
+<%= render(Primer::Box.new(classes: "account-prompt", border: :top, mt: 4, pt: 4, text_align: :center)) do %>
+  <%= render(Primer::Beta::Heading.new(tag: :h3, font_size: 4, font_weight: :bold, mb: 3)) do %>
+    <%= I18n.t("account.already_have_an_account") %>
+  <% end %>
+  <%= render(Primer::Beta::Link.new(href: signin_path, font_weight: :semibold)) do %>
+    <%= I18n.t(:button_login) %>
+  <% end %>
 <% end %>

app/views/account/_signup_prompt.html.erb

@@ -0,0 +1,36 @@
+<%#-- copyright
+OpenProject is an open source project management software.
+Copyright (C) the OpenProject GmbH
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+See COPYRIGHT and LICENSE files for more details.
+
+++#%>
+<%= render(Primer::Box.new(classes: "account-prompt", border: :top, mt: 4, pt: 4, text_align: :center)) do %>
+  <%= render(Primer::Beta::Heading.new(tag: :h3, font_size: 4, font_weight: :bold, mb: 3)) do %>
+    <%= I18n.t("account.no_account_yet") %>
+  <% end %>
+  <%= render(Primer::Beta::Link.new(href: account_register_path, font_weight: :semibold)) do %>
+    <%= I18n.t(:label_register) %>
+  <% end %>
+<% end %>

app/views/account/login.html.erb

@@ -32,13 +32,16 @@ See COPYRIGHT and LICENSE files for more details.
 
 <%= render Homescreen::AnnouncementComponent.new(announcement: Announcement.active_and_current) %>
 
-<div id="login-form" class="form -bordered">
-  <h1><%= I18n.t(:label_login) %></h1>
-
-  <hr class="form--separator">
+<div id="login-form" class="form">
+  <%= render(Primer::Beta::Heading.new(tag: :h1, font_size: 4, font_weight: :bold, text_align: :center, mb: 3)) do %>
+    <%= t("account.login_title", app_title: Setting.app_title) %>
+  <% end %>
   <% if show_password_login_form? %>
     <%= render partial: "password_login_form" %>
   <% end %>
   <%= render partial: "auth_providers" %>
+  <% if Setting::SelfRegistration.enabled? %>
+    <%= render partial: "signup_prompt" %>
+  <% end %>
 </div>
 <%= call_hook :view_account_login_bottom %>

app/views/account/register.html.erb

@@ -21,7 +21,7 @@ GNU General Public License for more details.
 
 You should have received a copy of the GNU General Public License
 along with this program; if not, write to the Free Software
-Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
 See COPYRIGHT and LICENSE files for more details.
 
@@ -30,8 +30,8 @@ See COPYRIGHT and LICENSE files for more details.
 <% html_title t(:label_register) %>
 <% @user ||= User.new %>
 
-<%= render(Primer::BaseComponent.new(tag: :div, classes: "op-account-registration")) do %>
-  <%= render(Primer::Beta::Text.new(tag: :p, classes: "op-account-registration--subtitle")) do %>
+<%= render(Primer::Box.new(classes: "op-account-registration")) do %>
+  <%= render(Primer::Beta::Heading.new(tag: :h1, font_size: 4, font_weight: :bold, text_align: :center, mb: 3)) do %>
     <%= t("account.signup_title", app_title: Setting.app_title) %>
   <% end %>
 
@@ -54,10 +54,8 @@ See COPYRIGHT and LICENSE files for more details.
   <% end %>
 
   <% unless @user.uses_external_authentication? %>
-    <%= render("account/auth_providers", omniauth_title: I18n.t("account.signup_with_external_account"), wide: true) %>
+    <%= render("account/auth_providers", omniauth_title: I18n.t("account.signup_with_external_account")) %>
   <% end %>
 
-  <div class="op-account-registration--signin">
-    <%= link_to t(:button_login), signin_path %>
-  </div>
+  <%= render partial: "account/signin_prompt" %>
 <% end %>

config/locales/en.yml

@@ -47,7 +47,9 @@ en:
         view: "A dynamic link to a view placed using a macro."
         work_package: "A dynamic link to a work package placed using a macro."
   account:
+    already_have_an_account: "You already have an account?"
     auth_source_login_html: Please login as <em>%{login}</em> to activate your account.
+    continue_with_provider: "Continue with %{name}"
     delete: "Delete account"
     delete_confirmation: "Are you sure you want to delete the account?"
     deletion_info:
@@ -72,7 +74,9 @@ en:
     error_self_registration_limited_provider: >
       User registration is limited for the Single sign-on provider '%{name}'. Please ask an administrator to activate the
       account for you or change the self registration limit for this provider.
-    login_with_auth_provider: "or sign in with your existing account"
+    login_title: "Sign in to %{app_title}"
+    login_with_external_account: "Sign in with an external account"
+    no_account_yet: "You don't have an account yet?"
     omniauth_direct_login_continue: "Continue to sign in"
     omniauth_direct_login_redirecting: "Redirecting you to your identity provider…"
     omniauth_login: Please login to activate your account.
@@ -452,7 +456,7 @@ en:
         last_name: "Last name"
         ldap_auth_source: "LDAP connection"
         new_password: "New password"
-        password_confirmation: "Confirmation"
+        password_confirmation: "Password confirmation"
       user_custom_field:
         custom_field_section: Section
       user_non_working_time: