<p></p>
<p><b>@tomhughes</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3257#discussion_r671874689">app/controllers/preferences_controller.rb</a>:</p>
<pre style='color:#555'>> +  layout "site"
+
+  before_action :authorize_web
+  before_action :set_locale
+
+  authorize_resource :class => false
+
+  before_action :check_database_readable
+  before_action :check_database_writable, :only => [:update]
+
+  def show; end
+
+  def edit; end
+
+  def update
+    current_user.languages = params[:user][:languages].split(",")
</pre>
<p>I think this needs a <code>set_locale</code> call to ensure that any changes are active? You removed it from the user settings but don't seem to have added it back here...</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3257#discussion_r671875396">app/views/preferences/edit.html.erb</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,12 @@
+<% content_for :heading do %>
+  <h1><%= t ".title" %></h1>
+<% end %>
+
+<%= bootstrap_form_for current_user, :url => { :action => :update } do |f| %>
+  <%= 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.text_field :languages %>
+
+  <%= f.primary t(".save") %>
+  <%= link_to t(".cancel"), preferences_path, :class => "btn btn-link" %>
</pre>
<p>Would <code>btn-outline-secondary</code> be a better a choice here? Having a link alongside a button looks a little odd to me?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3257#discussion_r671875632">app/views/profiles/edit.html.erb</a>:</p>
<pre style='color:#555'>> +    <div id="homerow" <% unless current_user.home_lat and current_user.home_lon %> class="nohome"<% end %>>
+      <p class="message text-muted"><%= t ".no home location" %></p>
+      <div class="form-row">
+        <%= 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" %>
+      </div>
+    </div>
+    <div class="form-check">
+      <input class="form-check-input" type="checkbox" name="updatehome" value="1" <% unless current_user.home_lat and current_user.home_lon %> checked="checked" <% end %> id="updatehome" />
+      <label class="form-check-label" for="updatehome"><%= t ".update home location on click" %></label>
+    </div>
+    <%= tag.div "", :id => "map", :class => "content_map set_location" %>
+  </fieldset>
+
+  <%= f.primary t(".save") %>
+    <%= link_to t(".cancel"), user_path(current_user), :class => "btn btn-link" %>
</pre>
<p>Same comment here about the class for the cancel button/link.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3257#discussion_r671875700">app/views/users/show.html.erb</a>:</p>
<pre style='color:#555'>> @@ -165,6 +165,12 @@
 
     <div class="user-description richtext text-break"><%= @user.description.to_html %></div>
 
+    <% if current_user and @user.id == current_user.id %>
+      <div class="my-3">
+        <%= link_to t(".edit_profile"), edit_profile_path, :class => "btn btn-outline-primary" %>
</pre>
<p>I'm not sure about the placement of this - it looks a bit odd to me. That may be partly because it's above the extra user data line that administrators like me get but I think it would stick out a bit without that.</p>
<p>Could we maybe put it on the right at the top? or would that be an issue on small screens?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3257#pullrequestreview-709048087">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLIPYSGXEDYEKU6J3HLTYMIABANCNFSM5AL23W7Q">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AAK2OLO6CVVXDO63MWFIA63TYMIABA5CNFSM5AL23W72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOFJBTOFY.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/3257#pullrequestreview-709048087",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/3257#pullrequestreview-709048087",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>