Add Map Button in Mobile View

[rajsudeep]

As requested in #867, provides a map button in the mobile view (window width <= 767) that when is toggled, displays the google map within the search list container.

[richardxia]

Hi @rajsudeep, thanks for submitting the PR!

I think, however, I'd prefer if we took a slightly different approach with this. Rather than handling the responsiveness in JavaScript, I would prefer to keep it in CSS as much as possible using media queries, since that's how all of the rest of the responsive design is encoded. In addition, it's more robust than attaching event handlers on resize operations, since the media queries operate directly on the viewport size.

For a rough overview of what I was thinking, I think I would wrap most of the HTML elements with a class that does the appropriate media query and hiding, and the only JavaScript logic would be to set some state that would attach some CSS classes that would override the media query. For example, the <button> that toggles the map can always present in the DOM, regardless of the viewport size, but it could have a CSS media query based on the screen width to actually show/hide it. Similarly, the actual map itself could have a media query that hides it on smaller screen sizes, but if the toggle map button has been pressed, then an extra CSS class could be added to it which always displays the map, even on smaller resolutions.

One last thing, although we don't really consistently adopt BEM, I think in this case it would be useful to think of the map toggling as a "Modifier" in BEM terminology, so rather than creating an entirely separate .results-map-mobile selector and duplicating the .results-map properties, you could instead unconditionally apply the .results-map class to the map object and conditionally add a modifier class when the map is toggled.

Since we haven't met, I'm not sure what your relative experience level with frontend technologies, but let me know if you wanted to chat for a bit about any of this stuff.

[richardxia]

I think this conditional will completely go away if you make more of the media queries in CSS, like I recommended, but I wanted to point out a general tip about stuff like this. Since the result of the conditional is to just set or unset a single prop on the component, you don't actually need the ternary expression around it and you can instead just pass a boolean value to isMobile. For example, this could just be written as:

<SearchMap
  hits={hits}
  page={searchResults.page}
  hitsPerPage={searchResults.hitsPerPage}
  isMobile={getMapButton && isMobile}
/>

[rajsudeep]

I see, thank you for the detailed input! I'll take a stab at implementing this using media queries if I get the time. Until then I'll leave this ticket open again on Clubhouse in case anyone else is able to get the job done before me. I'll be sure to contact you regarding any questions I have on implementing this. Cheers!

[richardxia]

Cool, thanks! Let me know if you ever wanted to spend a bit of time chatting about this and I'd be happy to help out.