<p></p>
<p dir="auto">This issue is a post-merge UI review for <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2064057773" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/4455" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/4455/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/4455">#4455</a> . cc <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/milan-cvetkovic/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/milan-cvetkovic">@milan-cvetkovic</a></p>
<p dir="auto">(I haven't done a post-merge review before, so I'm not sure if there's a better way to do it than to create a new issue - adding a review to a closed PR doesn't seem right either 🤷 )</p>
<h1 dir="auto">Globe positioning</h1>
<p dir="auto">The globe has been repositioned using an arbitrary offset. This appears to be to keep it clear of the tabs, but it doesn't take into account translations.</p>
<div class="highlight highlight-source-diff" dir="auto"><pre class="notranslate">   &.new-user-main {
     background-image: image-url("sign-up-illustration.png");
<span class="pl-mi1"><span class="pl-mi1">+</span>    background-position-x: 50px;</span>
   }</pre></div>
<p dir="auto"><a href="https://github.com/openstreetmap/openstreetmap-website/assets/360803/5c2dbf0a-254d-4756-ba92-a1451de846fc">Screenshot.from.2024-05-08.16-58-29.png (view on web)</a></p>
<p dir="auto">The offset should be removed, or designed to adapted to different sized tab labels.</p>
<h1 dir="auto">Page width</h1>
<div class="highlight highlight-source-diff" dir="auto"><pre class="notranslate"><span class="pl-mi1"><span class="pl-mi1">+</span> .auth-container {</span>
<span class="pl-mi1"><span class="pl-mi1">+</span>  max-width: 600px;</span>
<span class="pl-mi1"><span class="pl-mi1">+</span> }</span></pre></div>
<p dir="auto">This is a random width, not based on anything from the bootstrap grid. It makes the login and signup pages inconsistent with the rest of the site.</p>
<h1 dir="auto">Tab alignment</h1>
<p dir="auto">The edge of the tabs should be aligned with the edge of the content. See the traces page for a working example.<br>
<a href="https://github.com/openstreetmap/openstreetmap-website/assets/360803/43df826d-d8b8-4e0c-a696-d8d952d6e4f7">Screenshot.from.2024-05-08.17-13-35.png (view on web)</a></p>
<h1 dir="auto">Top menu buttons</h1>
<p dir="auto">The login/signup buttons disappear from the top menu in certain circumstances, and it leads to jarring inconsistencies. It means the rest of the menu moves around, and it also still doesn't work as intended (e.g. if you make a mistake while signing up, the signup page is shown with errors and the buttons reappear).</p>
<div class="highlight highlight-source-diff" dir="auto"><pre class="notranslate"><span class="pl-md"><span class="pl-md">-</span> <% else %></span>
<span class="pl-mi1"><span class="pl-mi1">+</span> <% elsif (controller_name != "users" and controller_name != "sessions") || action_name != "new" %></span></pre></div>
<p dir="auto">This should be reverted. If there's still a desire to remove these buttons (e.g. from the narrow screen layout) then let's discuss that separately.</p>
<h1 dir="auto">auth button sizes</h1>
<p dir="auto">The auth button sizes were reduced from 36px to 24px, but I didn't see any explanation why making them smaller is better?</p>
<h1 dir="auto">sizing for auth_button vs auth_button_preferred</h1>
<p dir="auto">In app/helpers/user_helper we no have two different ways to describe the size of the icon, these should be consistent:</p>
<pre class="notranslate"><code class="notranslate">                :size => "24"),
</code></pre>
<pre class="notranslate"><code class="notranslate">                :width => "24px",
                :height => "24px")
</code></pre>
<h1 dir="auto">bg-white</h1>
<p dir="auto">The "Log into OpenstreetMap to access..." panel uses <code class="notranslate">bg-white</code>, but text on a forced-white background doesn't work in dark mode<br>
<a href="https://github.com/openstreetmap/openstreetmap-website/assets/360803/444c8539-ec7c-491c-840e-6087ee7fbcd2">Screenshot.from.2024-05-08.15-44-24.png (view on web)</a></p>
<h1 dir="auto">auth_button_preferred css classes</h1>
<pre class="notranslate"><code class="notranslate">:class => "auth_button fs-6 border rounded text-muted text-decoration-none py-2 px-4 d-flex justify-content-center align-items-center",
</code></pre>
<p dir="auto"><a href="https://github.com/openstreetmap/openstreetmap-website/assets/360803/bc1c9f1d-46a4-4705-93e7-e46abe05ab30">Screenshot.from.2024-05-08.16-03-24.png (view on web)</a></p>
<p dir="auto">This list of classes is trying to style the link as a button, but either an actual button or at least a <code class="notranslate">btn</code> class should be used instead. This will ensure consistency with other buttons on forms etc. <code class="notranslate">.btn.btn-outline-secondary</code> could be a good place to start.</p>
<h1 dir="auto">auth_button_preferred alignment</h1>
<p dir="auto"><a href="https://github.com/openstreetmap/openstreetmap-website/assets/360803/a592e7a3-02ca-4428-8d2e-1ba536b8acee">Screenshot.from.2024-05-08.16-03-16.png (view on web)</a></p>
<p dir="auto">The alignment here isn't great. I think it might look better if the row of buttons was divided in the exact centre, with the preferred button then centered on the left half, and the remaining buttons centered on the right half. The current situation does have two divs displayed side-by-side, but with asymetric margins (mx-2 on one, and nothing on the other).</p>
<p dir="auto">They look good on narrow screens.</p>
<p dir="auto">If the design intention is just to have one set of buttons centred, then the gap between the preferred button and the remaining buttons is too large.</p>
<h1 dir="auto">"or" divider</h1>
<p dir="auto">The "or" divider looks cramped, too close to the form labels below, and could probably do with a standard mb-3 to space things out.</p>
<h1 dir="auto">"hr with words"</h1>
<p dir="auto">The PR introduced a new UI convention of using a horizontal line with words in the middle (e.g. "or" or "or sign up with a third party"). This has been implemented by using borders on empty divs, and with the same verbose code copy+pasted a few times:</p>
<pre class="notranslate"><code class="notranslate">    <div class="d-flex justify-content-center align-items-center">
      <div class="border-bottom border-1 flex-grow-1"></div>
      <div class="text-secondary mx-3"><%= t ".or" %></div>
      <div class="border-bottom border-1 flex-grow-1"></div>
    </div>
</code></pre>
<p dir="auto">If there are suggestions for a better way to implement this, that would be great. It's effectively a horizontal rule, or a section divider, and there might be a more semantic way to implement this, that we can reuse in other forms and in other places around the site. In particular, I don't think this implementation is accessible, since there's no meaningful markup surrounding what is then an isolated word.</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/issues/4773">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLOMUMFMSQPOSQQY24DZBJJK3AVCNFSM6AAAAABHNKIDOCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4DMMBSGM2TGNQ">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLPGC2MRAYZWYKJGYWDZBJJK3A5CNFSM6AAAAABHNKIDOCWGG33NNVSW45C7OR4XAZNFJFZXG5LFVJRW63LNMVXHIX3JMTHIQQPTOA.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/issues/4773</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/issues/4773",
"url": "https://github.com/openstreetmap/openstreetmap-website/issues/4773",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>