@hlfan commented on this pull request.


> @@ -75,10 +75,12 @@
         </div>
       </div>
     <% else %>
-      <div class="d-inline-flex btn-group login-menu">
-        <%= link_to t(".log_in"), login_path(:referer => request.fullpath), 
:class => "geolink btn btn-outline-secondary" %>
-        <%= link_to t(".sign_up"), new_user_path, :class => "btn 
btn-outline-secondary" %>
-      </div>
+      <% if allow_signup %>
+        <div class="d-inline-flex btn-group login-menu">
+          <%= link_to t(".log_in"), login_path(:referer => request.fullpath), 
:class => "geolink btn btn-outline-secondary" %>

Is it your intention to hide the login button for all pages unless your new 
parameter is present?
Because this might be why tests are failing:

<img 
src="https://github.com/user-attachments/assets/644c1e9d-104d-4034-abb2-f630322adb5e";
 />

Have you considered inverting the logic so that only pages with a 
`disallow_signup` parameter are changed?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7138#pullrequestreview-4445318570
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/7138/review/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to