From a7d959e5886d8bf3003e798b375bf5b4e644d18c Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 27 Jan 2021 15:36:59 +0000 Subject: [PATCH] Refactor account form to use bootstrap There's further refactoring that could be done, but this is sufficient for now. --- app/views/users/account.html.erb | 206 ++++++++-------------- config/locales/en.yml | 32 ++-- test/controllers/users_controller_test.rb | 40 ++--- 3 files changed, 100 insertions(+), 178 deletions(-) diff --git a/app/views/users/account.html.erb b/app/views/users/account.html.erb index 394e81cd1..4562033a2 100644 --- a/app/views/users/account.html.erb +++ b/app/views/users/account.html.erb @@ -10,158 +10,96 @@ <% end %> -<%= error_messages_for current_user %> -<%= form_for current_user, :url => { :action => :account }, :method => :post, :html => { :multipart => true, :id => "accountForm", :class => "standard-form", :autocomplete => :off } do |f| %> -
-
- - <%= f.text_field :display_name %> -
-
- -
-
- - - <%= t ".email never displayed publicly" %> -
- -
- - <%= f.email_field :new_email, :autocomplete => :off %> - <%= t ".email never displayed publicly" %> -
-
- -
-
- - <%= f.password_field :pass_crypt, :value => "", :autocomplete => :off %> -
- -
- - <%= f.password_field :pass_crypt_confirmation, :value => "", :autocomplete => :off %> +<%= bootstrap_form_for current_user, :url => { :action => :account }, :method => :post, :html => { :multipart => true, :id => "accountForm", :autocomplete => :off } do |f| %> + + <%= f.text_field :display_name %> + <%= f.email_field :email, :disabled => true, :label => t(".current email address") %> + <%= f.email_field :new_email, :autocomplete => :off %> + <%= f.password_field :pass_crypt, :value => "", :autocomplete => :off %> + <%= f.password_field :pass_crypt_confirmation, :value => "", :autocomplete => :off %> + +
+ +
+ <%= f.select(:auth_provider, Auth.providers, :hide_label => "true", :wrapper => { :class => "col-auto mb-0" }) %> + <%= f.text_field(:auth_uid, :hide_label => true, :wrapper => { :class => "col mb-0" }) %>
+ (" target="_new"><%= t ".openid.link text" %>)
-
-
- - <%= f.select :auth_provider, Auth.providers %> - <%= f.text_field :auth_uid %> - (" target="_new"><%= t ".openid.link text" %>) -
-
- -
-
- - - <% if current_user.data_public? %> - <%= t ".public editing.enabled" %> - (" target="_new"><%= t ".public editing.enabled link text" %>) - <% else %> - <%= t ".public editing.disabled" %> - (<%= t ".public editing.disabled link text" %>) - <% end %> - -
- -
- - - <% if current_user.terms_agreed? %> - <%= t ".contributor terms.agreed" %> - (" target="_new"><%= t ".contributor terms.link text" %>) - <% if current_user.consider_pd? %> - <%= t ".contributor terms.agreed_with_pd" %> - <% end %> - <% else %> - <%= t ".contributor terms.not yet agreed" %> - <%= link_to t(".contributor terms.review link text"), :controller => "users", :action => "terms" %> +
+ + + <% if current_user.data_public? %> + <%= t ".public editing.enabled" %> + (" target="_new"><%= t ".public editing.enabled link text" %>) + <% else %> + <%= t ".public editing.disabled" %> + (<%= t ".public editing.disabled link text" %>) + <% end %> + +
+ +
+ + + <% if current_user.terms_agreed? %> + <%= t ".contributor terms.agreed" %> + (" target="_new"><%= t ".contributor terms.link text" %>) + <% if current_user.consider_pd? %> + <%= t ".contributor terms.agreed_with_pd" %> <% end %> - -
-
- - <%= f.select :preferred_editor, [[t("editor.default", :name => t("editor.#{Settings.default_editor}.name")), "default"]] + Editors::AVAILABLE_EDITORS.collect { |e| [t("editor.#{e}.description"), e] } %> -
-
- -
-
- - <%= richtext_area :user, :description, :object => current_user, :cols => 80, :rows => 20 %> -
- -
- - <%= f.text_field :languages %> -
- -
- + <% else %> + <%= t ".contributor terms.not yet agreed" %> + <%= link_to t(".contributor terms.review link text"), :controller => "users", :action => "terms" %> + <% end %> + +
+ + <%= f.select :preferred_editor, [[t("editor.default", :name => t("editor.#{Settings.default_editor}.name")), "default"]] + Editors::AVAILABLE_EDITORS.collect { |e| [t("editor.#{e}.description"), e] } %> + <%= f.richtext_field :description, :cols => 80, :rows => 20 %> + <%= f.text_field :languages %> + +
+ <%= f.label t(".image") %> +
+
<%= user_image current_user %> -
    +
+
<% if current_user.avatar.attached? %> -
  • - <%= radio_button_tag "avatar_action", "keep", !current_user.image_use_gravatar %> - -
  • + <%= f.radio_button "avatar_action", "keep", :name => "avatar_action", :label => t(".keep image"), :checked => !current_user.image_use_gravatar %> <% end %> <% if current_user.avatar.attached? || current_user.image_use_gravatar? %> -
  • - <%= radio_button_tag "avatar_action", "delete" %> - -
  • + <%= f.radio_button "avatar_action", "delete", :name => "avatar_action", :label => t(".delete image"), :checked => false %> <% end %> <% if current_user.avatar.attached? %> -
  • - <%= radio_button_tag "avatar_action", "new" %> - - <%= f.file_field :avatar %> -
  • + <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %> + <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".replace image"), :checked => false %> + <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %> + <% end %> <% else %> -
  • - <%= radio_button_tag "avatar_action", "new" %> - - <%= f.file_field :avatar %> -
  • + <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %> + <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".new image"), :checked => false %> + <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %> + <% end %> + <% end %> + <%= f.form_group :help => link_to(t(".gravatar.what_is_gravatar"), t(".gravatar.link")) do %> + <%= f.radio_button "avatar_action", "gravatar", :name => "avatar_action", :label => t(".gravatar.gravatar"), :checked => current_user.image_use_gravatar %> <% end %> -
  • - <%= radio_button_tag "avatar_action", "gravatar", current_user.image_use_gravatar %> - -
  • - +
    -
    -
    - +
    + <%= t ".home location" -%>
    class="nohome"<% end %>>

    <%= t ".no home location" %>

    -
    - - <%= f.text_field :home_lat, :id => "home_lat" %> -
    -
    - - <%= f.text_field :home_lon, :id => "home_lon" %> -
    +
    + <%= f.text_field :home_lat, :wrapper_class => "col-sm-4", :id => "home_lat" %> + <%= f.text_field :home_lon, :wrapper_class => "col-sm-4", :id => "home_lon" %>
    -
    checked="checked" <% end %> id="updatehome" /> @@ -169,7 +107,7 @@ <%= tag.div "", :id => "map", :class => "content_map set_location" %>
    - <%= submit_tag t(".save changes button") %> + <%= f.primary t(".save changes button") %> <% end %> <% unless current_user.data_public? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index c28492a27..da86c283d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -122,10 +122,14 @@ en: user: email: "Email" email_confirmation: "Email Confirmation" + new_email: "New Email Address" active: "Active" display_name: "Display Name" - description: "Description" - languages: "Languages" + description: Profile Description + home_lat: Latitude + home_lon: Longitude + languages: Preferred Languages + preferred_editor: Preferred Editor pass_crypt: "Password" pass_crypt_confirmation: "Confirm Password" help: @@ -136,6 +140,7 @@ en: needs_view: Does the user need to log in before this block will be cleared? user: email_confirmation: 'Your address is not displayed publicly, see our privacy policy for more information.' + new_email: "(never displayed publicly)" datetime: distance_in_words_ago: about_x_hours: @@ -2308,8 +2313,6 @@ en: display name: "Display Name:" display name description: "Your publicly displayed username. You can change this later in the preferences." external auth: "Third Party Authentication:" - password: "Password:" - confirm password: "Confirm Password:" use external auth: "Alternatively, use a third party to login" auth no password: "With third party authentication a password is not required, but some extra tools or server may still need one." continue: Sign Up @@ -2414,15 +2417,13 @@ en: account: title: "Edit account" my settings: My settings - current email address: "Current Email Address:" - new email address: "New Email Address:" - email never displayed publicly: "(never displayed publicly)" - external auth: "External Authentication:" + current email address: "Current Email Address" + external auth: "External Authentication" openid: link: "https://wiki.openstreetmap.org/wiki/OpenID" link text: "what is this?" public editing: - heading: "Public editing:" + heading: "Public editing" enabled: "Enabled. Not anonymous and can edit data." enabled link: "https://wiki.openstreetmap.org/wiki/Anonymous_edits" enabled link text: "what is this?" @@ -2432,21 +2433,18 @@ en: heading: "Public editing" html: "Currently your edits are anonymous and people cannot send you messages or see your location. To show what you edited and allow people to contact you through the website, click the button below. Since the 0.6 API changeover, only public users can edit map data. (find out why).
    • Your email address will not be revealed by becoming public.
    • This action cannot be reversed and all new users are now public by default.
    " contributor terms: - heading: "Contributor Terms:" + heading: "Contributor Terms" agreed: "You have agreed to the new Contributor Terms." not yet agreed: "You have not yet agreed to the new Contributor Terms." review link text: "Please follow this link at your convenience to review and accept the new Contributor Terms." agreed_with_pd: "You have also declared that you consider your edits to be in the Public Domain." link: "https://www.osmfoundation.org/wiki/License/Contributor_Terms" link text: "what is this?" - profile description: "Profile Description:" - preferred languages: "Preferred Languages:" - preferred editor: "Preferred Editor:" - image: "Image:" + image: Image gravatar: gravatar: "Use Gravatar" link: "https://wiki.openstreetmap.org/wiki/Gravatar" - link text: "what is this?" + what_is_gravatar: "What is Gravatar?" disabled: "Gravatar has been disabled." enabled: "Display of your Gravatar has been enabled." new image: "Add an image" @@ -2454,10 +2452,8 @@ en: delete image: "Remove the current image" replace image: "Replace the current image" image size hint: "(square images at least 100x100 work best)" - home location: "Home Location:" + home location: "Home Location" no home location: "You have not entered your home location." - latitude: "Latitude:" - longitude: "Longitude:" update home location on click: "Update home location when I click on the map?" save changes button: Save Changes make edits public button: Make all my edits public diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index c21f82046..03657d288 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -958,9 +958,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row > div#user_description_container > div#user_description_content > textarea#user_description", user.description + assert_select "form#accountForm > div.form-group > div#user_description_container > div#user_description_content > textarea#user_description", user.description # Changing to a invalid editor should fail user.preferred_editor = "unknown" @@ -968,8 +967,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template :account assert_select ".notice", false - assert_select "div#errorExplanation" - assert_select "form#accountForm > fieldset > div.standard-form-row > select#user_preferred_editor > option[selected]", false + assert_select "form#accountForm > div.form-group > select#user_preferred_editor > option[selected]", false # Changing to a valid editor should work user.preferred_editor = "id" @@ -979,9 +977,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row > select#user_preferred_editor > option[selected][value=?]", "id" + assert_select "form#accountForm > div.form-group > select#user_preferred_editor > option[selected][value=?]", "id" # Changing to the default editor should work user.preferred_editor = "default" @@ -991,9 +988,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row > select#user_preferred_editor > option[selected]", false + assert_select "form#accountForm > div.form-group > select#user_preferred_editor > option[selected]", false # Changing to an uploaded image should work image = Rack::Test::UploadedFile.new("test/gpx/fixtures/a.gif", "image/gif") @@ -1003,9 +999,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row.accountImage input[name=avatar_action][checked][value=?]", "keep" + assert_select "form#accountForm > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked][value=?]", "keep" # Changing to a gravatar image should work post user_account_path(user), :params => { :avatar_action => "gravatar", :user => user.attributes } @@ -1014,9 +1009,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row.accountImage input[name=avatar_action][checked][value=?]", "gravatar" + assert_select "form#accountForm > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked][value=?]", "gravatar" # Removing the image should work post user_account_path(user), :params => { :avatar_action => "delete", :user => user.attributes } @@ -1025,9 +1019,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row.accountImage input[name=avatar_action][checked]", false + assert_select "form#accountForm > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked]", false + assert_select "form#accountForm > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked]", false # Adding external authentication should redirect to the auth provider post user_account_path(user), :params => { :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") } @@ -1040,8 +1034,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template :account assert_select ".notice", false - assert_select "div#errorExplanation" - assert_select "form#accountForm > fieldset > div.standard-form-row > input.field_with_errors#user_display_name" + assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name" # Changing name to one that exists should fail, regardless of case new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name.upcase) @@ -1049,8 +1042,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template :account assert_select ".notice", false - assert_select "div#errorExplanation" - assert_select "form#accountForm > fieldset > div.standard-form-row > input.field_with_errors#user_display_name" + assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name" # Changing name to one that doesn't exist should work new_attributes = user.attributes.dup.merge(:display_name => "new tester") @@ -1060,9 +1052,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(:display_name => "new tester") assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row > input#user_display_name[value=?]", "new tester" + assert_select "form#accountForm > div.form-group > input#user_display_name[value=?]", "new tester" # Record the change of name user.display_name = "new tester" @@ -1077,8 +1068,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template :account assert_select ".notice", false - assert_select "div#errorExplanation" - assert_select "form#accountForm > fieldset > div.standard-form-row > input.field_with_errors#user_new_email" + assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email" # Changing email to one that exists should fail, regardless of case user.new_email = create(:user).email.upcase @@ -1090,8 +1080,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template :account assert_select ".notice", false - assert_select "div#errorExplanation" - assert_select "form#accountForm > fieldset > div.standard-form-row > input.field_with_errors#user_new_email" + assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email" # Changing email to one that doesn't exist should work user.new_email = "new_tester@example.com" @@ -1105,9 +1094,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest get user_account_path(user) assert_response :success assert_template :account - assert_select "div#errorExplanation", false assert_select ".notice", /^User information updated successfully/ - assert_select "form#accountForm > fieldset > div.standard-form-row > input#user_new_email[value=?]", user.new_email + assert_select "form#accountForm > div.form-group > input#user_new_email[value=?]", user.new_email email = ActionMailer::Base.deliveries.first assert_equal 1, email.to.count assert_equal user.new_email, email.to.first -- 2.39.5