Conversation
… info section if additional info was an empty list. Adjusted lined list item CSS to overlap the line a little bit less so that the focus outline isn't as wonky. Unfortunately, getting rid of the focus outline wonk entirely brings back the little gap issue.
| class="flex items-center gap-2" | ||
| > | ||
| <div class="h-0 invisible shrink-0"> | ||
| <RouteComponents.route_icon size="small" route={@route} /> |
There was a problem hiding this comment.
Why do we have an invisible icon here? This was the root element of the problem and the fact that it was rendering even with an empty additional_info. Can we remove it?
There was a problem hiding this comment.
The invisible icon makes it so that Train 991 • Track 1 (Outbound) is aligned with Stoughton in the screenshot below.
I don't mean that that's the right way to do it 😅 - just that that's what I came up with to solve that problem when I originally built this!
I'm guessing that the Correct™ answer here involves using a CSS grid, but I vaguely remember struggling with line-wrapping - with the grid either pre-emptively wrapping all the lines, or refusing to line-wrap at all ever, even if the screen was narrow.
(This is pretty much the line-wrapping we want, which we got for free with flex)
I'm all for trying again to get this right though - either as part of this PR or a follow-up!
| class="flex items-center gap-2" | ||
| > | ||
| <div class="h-0 invisible shrink-0"> | ||
| <RouteComponents.route_icon size="small" route={@route} /> |
There was a problem hiding this comment.
The invisible icon makes it so that Train 991 • Track 1 (Outbound) is aligned with Stoughton in the screenshot below.
I don't mean that that's the right way to do it 😅 - just that that's what I came up with to solve that problem when I originally built this!
I'm guessing that the Correct™ answer here involves using a CSS grid, but I vaguely remember struggling with line-wrapping - with the grid either pre-emptively wrapping all the lines, or refusing to line-wrap at all ever, even if the screen was narrow.
(This is pretty much the line-wrapping we want, which we got for free with flex)
I'm all for trying again to get this right though - either as part of this PR or a follow-up!
| <div | ||
| class="w-6 shrink-0 self-stretch flex justify-center relative" | ||
| style="margin-block: calc(-1 * (var(--spacing-3) + 1px));" | ||
| style="margin-block: calc(-1 * (var(--spacing-3) + 0.5px));" |
There was a problem hiding this comment.
This actually makes the gap issue more pronounced, and has it show up for smaller font sizes.
This is with a font size of 24px, where the gaps show up (quite small, but very visible if you view the image at full size) on this branch, but not on prod. On prod, the gaps only show up for font sizes >= 40px.
The root cause here, definitely of the gap issue, and maybe of the funky outline as well, is that we're adding a value in px to a value in rem, which usually leads to funky results. (There are very rare circumstances in which it's the right call, but I wouldn't be surprised if that basically never happens on dotcom.)
I bet we could solve the gap issue and make the outline be completely non-funky by changing the 0.5px to a value in rem instead. (0.03125rem is the same as 0.5px at the 16px font size, but you may be able to get to fully non-funky by fiddling with the exact value).
joshlarson
left a comment
joshlarson
left a comment
There was a problem hiding this comment.
A few thoughts on px versus rem, and some historical context for my initial (probably not very good) attempt to align things!
2 participants
Scope
Asana Ticket: 📅🔎 Fix funky focus outline
Implementation
Adjusted schedule finder logic so it wouldn't render an empty additional_info section if additional_info was an empty list.
Adjusted lined_list_item CSS to overlap the line a little bit less so that the focus outline isn't as wonky. Unfortunately, getting rid of the focus outline wonk entirely brings back the little gap issue.
Screenshots
🚇 Subway
🚂 Commuter Rail
🚌 Bus
How to test
http://localhost:4001/departures/?route_id=71&direction_id=1&stop_id=2052
Check out various lines and modes and observe that the focus outlines look much better than before.