feat: Arrange map and plan results correctly on desktop and mobile #2240
Conversation
| @@ -181,10 +181,6 @@ $mobile-control-height: 73px; | |||
| cursor: pointer; | |||
| } | |||
|
|
|||
| .hidden { | |||
| display: none !important; | |||
There was a problem hiding this comment.
This feels like it could be a big risky change, but also this utility was overriding the tailwind utility that does the same thing (except that the tailwind utility doesn't have !important anymore, since we removed that here).
Interested in your thoughts on getting rid of this, and whether there's another way to get tailwind to conditionally give us display: none in the right settings.
| target={@myself} | ||
| /> | ||
| <div> | ||
| <section class={"flex flex-col md:grid #{@grid_column_layout} md:grid-rows-[min-content_1fr] w-full border border-solid border-slate-400"}> |
There was a problem hiding this comment.
I could also see a world in which this was a grid on both desktop and mobile, just with a different template, and different row-start/row-span/etc for the grid entries, but this felt easier as I was building it. This way, lexically the pieces are arranged according to the mobile design, and then on desktop, they get placed onto the grid instead, and their lexical ordering is ignored.
Open to feedback/ideas on this.
| /> | ||
| </section> | ||
| <.async_result :let={results} assign={@results}> | ||
| <div |
There was a problem hiding this comment.
This added some extra spacing between View all options and the content below it.
I'm currently leaning towards leaving it as-is for this PR, and fixing it, as well as a bunch of other minor visual issues as a follow-up.
| ~H""" | ||
| <section class="flex w-full border border-solid border-slate-400"> | ||
| <section class={"flex flex-col md:grid #{@grid_column_layout} md:grid-rows-[min-content_1fr] w-full border border-solid border-slate-400"}> |
There was a problem hiding this comment.
I could also see a world in which this was a grid on both desktop and mobile, just with a different template, and different row-start/row-span/etc for the grid entries, but this felt easier as I was building it. This way, lexically the pieces are arranged according to the mobile design, and then on desktop, they get placed onto the grid instead, and their lexical ordering is ignored.
Open to feedback/ideas on this.
(This was originally this comment, but got outdated when I changed the spacing, and is still relevant)
There was a problem hiding this comment.
This is probably a cleaner way to optionally assign classes than using case statements: https://github.com/mbta/mbta_metro/blob/8c01cc9b8bbd6018daea506b19a24e7b10ae95ae/lib/mbta_metro/components/flash.ex#L46
There was a problem hiding this comment.
Oh yeah, that is cleaner.
| class="row-span-2" | ||
| /> | ||
|
|
||
| <.async_result :let={results} assign={@results}> |
There was a problem hiding this comment.
Something about how I re-arranged the DOM elements added some extra spacing between View all options and the content below it.
I'm currently leaning towards leaving it as-is for this PR, and fixing it, as well as a bunch of other minor visual issues as a follow-up.
| ~H""" | ||
| <section class="flex w-full border border-solid border-slate-400"> | ||
| <section class={"flex flex-col md:grid #{@grid_column_layout} md:grid-rows-[min-content_1fr] w-full border border-solid border-slate-400"}> |
There was a problem hiding this comment.
This is probably a cleaner way to optionally assign classes than using case statements: https://github.com/mbta/mbta_metro/blob/8c01cc9b8bbd6018daea506b19a24e7b10ae95ae/lib/mbta_metro/components/flash.ex#L46
| map_config={@map_config} | ||
| from={@from} | ||
| to={@to} | ||
| hide_on_mobile={@expanded_itinerary_index == nil} |
There was a problem hiding this comment.
Instead of wrapping the map in another component and passing in this attribute, you could use the same ternary trick from above:
~H"""
<.live_component
module={MbtaMetro.Live.Map}
id="trip-planner-map"
class={[
"h-64 md:h-96 w-full relative overflow-none",
@expanded_itinerary_index == nil && "hidden md:block"
]}
config={@map_config}
pins={[@from, @to]}
/>
"""
Mobile
Before
After
Desktop
Looks pretty much the same
Asana Ticket: Trip Planner Preview | Scaffold out the responsive mobile/desktop layout