<p></p>
<p><b>@gravitystorm</b> requested changes on this pull request.</p>

<p dir="auto">I've added a couple of design comments that stand out to me, but there's plenty more here for me to review later.</p><hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#discussion_r1474138598">app/assets/stylesheets/common.scss</a>:</p>
<pre style='color:#555'>> @@ -1086,4 +1086,102 @@ div.secondary-actions {
   }
 }
 
+.auth-container {
</pre>
<p dir="auto">I haven't had the chance for a full review, but these stylesheet changes stand out to me. Our general approach is to use the bootstrap framework, not custom CSS. In particular, custom max-widths, custom font sizes, custom border radii etc aren't the way we should be implementing changes.</p>
<p dir="auto">Instead, bootstrap classes should be added to html elements, e.g. <code class="notranslate">rounded-2</code> should be used. These can of course be used sparingly, since html elements like <code class="notranslate"><small></code> should do the right thing too.</p>
<p dir="auto">I can see an implementation of tabs here too, and I would expect instead to see bootstrap tabs being used, as elsewhere on the site.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#discussion_r1474151891">app/views/users/new.html.erb</a>:</p>
<pre style='color:#555'>>  <% end %>
 
-<% content_for :heading_class, "pb-0" %>
-<% content_for :heading do %>
-  <div class='header-illustration new-user-main'>
</pre>
<p dir="auto">I'm surprised to see the header-illustration being removed from the signup pages, since it's one of the few pieces of genuinely original design in our project! But it's more surprising to see it removed from just this page, and not from the other pages that share the same design.</p>
<p dir="auto">What's the design rationale, if any, behind removing it?</p>
<p dir="auto">We don't have much in-house capacity for this kind of design, so it concerns me that whenever external companies come in, they often focus on these few pages (login signup etc) and keep redesigning over the top of whoever made the last external design changes. This leads to no redesign being anywhere near comprehensive, and each redesign not quite covering the same (small) ground as the previous one, and we end up with widespread inconsistencies.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4455#pullrequestreview-1856038628">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLKZP72KSXCK4OJYEIDYRNRHZAVCNFSM6AAAAABBLOL2OWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJWGAZTQNRSHA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLJ7FNPA45LSG2SRFRTYRNRHZA5CNFSM6AAAAABBLOL2OWWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTTOUDTOI.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/4455/review/1856038628</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/4455#pullrequestreview-1856038628",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4455#pullrequestreview-1856038628",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>