Skip to content

Commit

Permalink
More CSS tweaks for mobile tiled layout and general cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
Fred Mueller committed Jan 14, 2020
1 parent c81a820 commit 8f9927a
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 107 deletions.
21 changes: 16 additions & 5 deletions src/App.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const queryString = require('query-string');
*/

// const deviceIsMobile = isMobile; // HACK to allow easy mocking of isMobile for testing/debugging
const deviceIsMobile = true;
const deviceIsMobile = true; // HACK to allow easy mocking of isMobile for testing/debugging

function App() {

Expand Down Expand Up @@ -95,19 +95,31 @@ function App() {
}
}, [currZip, currRange]);

// Filters the events when there are new events from the API or when user changes filtering criteria
/*
* Filters the events when there are new events from the API or
* when the user changes filtering criteria
*/
useEffect(() => {

if (!events){
return setFilteredEvents(null);
setCardIndex(null); // no events, hence no valid cardIndex
setFilteredEvents(null); // no events, hence no filtered events
return;
}

setCardIndex(0); // reset to the first event in the list

// if ALLEVENTS then return everything, otherwise filter on the current kind of event
setFilteredEvents(events.filter((event) => {
return ((currEventKind === 'ALLEVENTS') || (currEventKind === event['event_type']));
}));
}, [events, currEventKind]);


// If the cardIndex changes, reset the event whose marker is highlighted, by calling setHoverEvent()
/*
* If the cardIndex changes, then reset the event whose
* marker is highlighted, by calling setHoverEvent()
*/
useEffect(() => {
if (deviceIsMobile && filteredEvents != null) {
setHoverEvent(
Expand All @@ -123,7 +135,6 @@ function App() {
// code just operates on the list of events passed to it.

return (
/* <div className="app"> */ /* frm: original code */
<div className={deviceIsMobile ? "app appIsMobile" : "app appIsDesktop"}>
<SearchBar currZip={currZip} currRange={currRange} currEventKind={currEventKind} updateZip={(newZip) => setCurrZip(newZip)} updateRange={(newRange) => setCurrRange(newRange)} updateEventKind={(newEventKind) => setCurrEventKind(newEventKind)} events={filteredEvents} updatedHover={(newHover) => setHoverEvent(newHover)} locFilt={locFilt} deviceIsMobile={deviceIsMobile}/>
{events === null && currZip == null &&
Expand Down
141 changes: 55 additions & 86 deletions src/App.scss
View file
Original file line number Diff line number Diff line change
@@ -1,65 +1,51 @@
/* frm: I added classes to distinguish between desktop and
* mobile cases - appIsDesktop and appIsMobile. This
* allowed me to set up the grid differently for the
* two cases. For desktop, the app has a left panel
* containing the search bar with the map on the
* right. For mobile, there is just a single column
* withy three rows (search, map, mobile-list-of-events).
/*
*
* The app uses a tiled layout that is different for desktop
* vs. mobile.
*
* On desktop, there are two columns: the left
* column contains the search criteria <div> above a
* scrolling list of events that match the search criteria.
* The width of the first column is constrained. The right
* column contains the map and it grows/shrinks to use up
* all of the remaining available space.
*
* On mobile, there is a single column that contains (in
* order from top to bottom): the search criteria then
* the map and then an area where the event details are
* provided along with buttons (previous, Info/RSVP, next).
*
* As you will see, we use CSS grid to implement this layout.
*
* To distinguish between the desktop and mobile cases, there
* is a class defined for each case: "appIsDesktop" and
* "appIsMobile".
*
*/

.appIsDesktop {
display: grid; /* frm: added for new map layout UI */
display: grid; /* on desktop use a two column grid layout - right column grows/shrinks */
grid-template-columns: minmax(180px,300px) auto;

#map{
height: 100vh; /* frm: changed from % to vh */
height: 100vh;
z-index: 9;
}

}

.appIsMobile {
display: grid; /* frm: added for new map layout UI */
display: grid; /* on mobile use a row grid with three rows: search, map, card & nav */
grid-template-rows: min-content auto min-content;

#map{
/* height: 100vh; */ /* frm: don't need height for mobile */
/* ??? frm:
* Actually I might need the height for mobile. The documentation
* for leaflet says that there needs to be a defined height.
*/
z-index: 9; /* frm: not sure z-index does anything for mobile */
}

}

/* frm: I added a <div class="mobileNavWrapper> around the mobile
* previous, rsvp, next buttons in order to get the positioning
* of the buttons to work out. There is probably a way to do it
* without resorting to creating their own container, but I
* didn't know how to do it. In any event, the only reason for
* this <div> is to get the positioning of the buttons
* to be left, center, right.
*/

/* frm: commented this out for now - replaced with code immediately below
*
.mobileNavWrapper {
** position: relative; ** ** frm: using flex below means I don't need relative position **
margin-top: 5px;
margin-bottom: 5px;
}
*
*/

.mobileNavWrapper { /* frm: new code based on flex */
display: flex;
flex-direction: row;
justify-content: space-around;
}

.app{
height: 100vh; /* frm: added for new map layout UI */
height: 100vh;

#startLoad{
position: fixed;
Expand Down Expand Up @@ -126,31 +112,17 @@ html, body, #root, .app {
}

.searchBar{
/* padding-top: 2%; */ /* frm: commented out for new map layout UI */
/* padding-bottom: 2%; */ /* frm: commented out for new map layout UI */
background-color: #232444;
/* max-width: 300px; */ /* frm: commented out for new map layout UI */
/* width: 32%; */ /* frm: commented out for new map layout UI */
/* position: fixed; */ /* frm: commented out for new map layout UI */
/* left: 2%; */ /* frm: commented out for new map layout UI */
/* top: 2%; */ /* frm: commented out for new map layout UI */
z-index:11; /* frm: not sure we need z-index anymore */
box-shadow: 10px 0px 5px rgba(0, 0, 0, .3); /* frm: don't need bottom shadow for new map layout UI */

box-shadow: 10px 0px 5px rgba(0, 0, 0, .3);
}

/* frm: Added class, desktopSearch, as an analog for the preexisting mobileSearch
* so that I could have a different layout (using grid) for the deesktop
* use case. In this case the search bar has two rows that take up all
* of the vertical space.
*/

.desktopSearch {
/* For desktop, we want two rows */
height: 100vh; /* frm: added for new map layout UI */
display: grid; /* frm: added for new map layout UI */
grid-template-rows: auto auto; /* frm: added for new map layout UI */
grid-gap: 0; /* frm: added for new map layout UI */
/* For desktop, we want two rows: 1) search criteria and 2) the scrolling list of events */
height: 100vh;
display: grid;
grid-template-rows: auto auto;
grid-gap: 0;
}

.mobileSearch {
Expand Down Expand Up @@ -193,7 +165,7 @@ html, body, #root, .app {
align-items: flex-end;
justify-content: center;
padding-left: 5%;
padding-right: 5%; /* frm: changed from 7% to 5% */
padding-right: 5%;
padding-top: 5%;
padding-bottom: 5%;

Expand Down Expand Up @@ -294,7 +266,7 @@ html, body, #root, .app {
font-size: 16px;
background-color: #f5f5f5;
border: none;
padding: 0px 5px 0px 5px; /* frm: changed right-pad from 20px to 5px */
padding: 0px 5px 0px 5px;
appearance: none;
-moz-appearance: none;
-webkit-appearance: none;
Expand Down Expand Up @@ -342,7 +314,6 @@ html, body, #root, .app {

.eventList{
background-color: white;
/* height: 500px; */ /* frm: commented out for new map layout UI */
overflow:hidden;
overflow-y:scroll;
margin-bottom: 0px;
Expand Down Expand Up @@ -403,26 +374,22 @@ html, body, #root, .app {

.mobileList{
margin: none;
font-size:calc(10px + 1vw); /* frm: changed from 12px to 10px to conserve whitespace */
font-size:calc(10px + 1vw);
touch-action: manipulation;
position: relative; /* frm: added so children can be positioned absolutely */

.eventCard{

text-decoration: none;
color: #232444;
/* position: fixed; */ /* frm: commented out for new map layout UI */
z-index: 10;
bottom: 10%; /* changed from 13% to 10% */
bottom: 10%;
background-color: white;
width: 90%;
left: 5%;
max-height: 30vh;
min-height: 10vh; /* changed from 20vh to 10vh to conserve whitespace */
box-shadow: 1px 2px 5px rgba(0, 0, 0, .3);
/* border: 1px solid #232444; */ /* frm: removed as part of new layout UI */
min-height: 10vh;

h3{
margin-top: 10px; /* frm: smaller for mobile to conserve whitespace */
margin-top: 10px;
margin-bottom: 10px;
text-transform: uppercase;
}
Expand All @@ -432,45 +399,47 @@ html, body, #root, .app {
}

.mobileInfo{

padding-left: 5%;
padding-right: 5%;
padding-bottom: 1%;

.eventRSVP{
display: none; /* frm: changed from visibility: hidden to display: none to conserve whitespace */
text-align: right;
display: none;
}

}
}

.mobileNavWrapper { /* contains navigation buttons (next/prev) and Info/RSVP button */
display: flex;
flex-direction: row;
justify-content: space-around;
}

button{
/* position: absolute; */ /* frm: changed for new map layout UI */
bottom: 3%; /* frm: changed from 5% to 3% */
bottom: 3%;
z-index: 10;
background-color: #232444;
border: none;
color: white;
font-size: 16px; /* frm: changed from 24px */
height: 30px; /* frm: changed from 40px to 30px */
box-shadow: 1px 2px 5px rgba(0, 0, 0, .3);
font-size: 16px;
height: 30px;

}
#mobileRSVP{
/* left: 35%; */ /* frm: commented out trying to get flex to work */
/* width: 30vw; */ /* frm: commented out because I now have it flex its width */
flex: 1;
flex: 1; /* Info/RSVP button stretches to use up all available space */
a{
text-decoration: none;
color:white;
}
}
#leftIndex{
/* left: 25px; */ /* frm: commented out trying to get flex to work */
width: 50px;
border-right: 3px solid white; /* separate nav button from RSVP/Info button */
}
#rightIndex{
/* right: 25px; */ /* frm: commented out trying to get flex to work */
width: 50px;
border-left: 3px solid white; /* separate nav button from RSVP/Info button */
}
}

Expand Down
13 changes: 1 addition & 12 deletions src/EventList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,6 @@ function EventTimes(props) {
}
}

/*
*
* ??? frm: DELETE this once I have verified that Util().eventHasValidLocation() works
*
function eventHasValidLocation(event) {
// Utility function to avoid repeating this logic...
return ('location' in event && 'location' in event['location'] && 'latitude' in event['location']['location']);
}
*
*/

export function EventList(props) {
let listEvents;
if(props.events.length > 0){
Expand Down Expand Up @@ -116,7 +105,7 @@ export function EventList(props) {
listEvents = null;
}

// frm: At this point listEvents is either null or the HTML for a list of each of the events
// At this point listEvents is either null or the HTML for a list of each of the events

return (
<ul className="eventList">{listEvents}
Expand Down
23 changes: 23 additions & 0 deletions src/Map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,30 @@ export function Map(props){

}

/* ??? frm: The current code immediately below does not correctly resize and
* rezoom on mobile. The problem is that the amount of space used
* for each card varies, and hence the amount of space available
* for the map can change every time the cardIndex changes.
*
* The code below tells the map to resize (by calling invlidatesize() )
* everytime the hoverMarker changes which is almost correct. Instead
* it should resize every time the cardIndex changes.
*
* Unfortuately, the Map currently does not know about the cardIndex
* It would be easy to pass it in the way the hoverMarker is currently
* passed in, but I want to wait and make that change in a separate
* pull request (there is another related issue/bug concerning hoverMarker
* on mobile that I will change at the same time).
*
* I am also not going to create an issue for this on github yet
* because this is not a problem in the old UI - so it doesn't make
* sense to create an issue that is not yet a problem in production.
* I will create an issue once the new tiled layout is merged...
*/

// zoom to marker bounds, plus padding to make sure entire marker is visible
// ??? frm: probably only have to invalidateSize() on mobile... (but it is a cheap op)
map.current.invalidateSize(); // make sure the map fits its allocated space (mobile issue)
map.current.fitBounds(markers.current.getBounds().pad(0.1));
}
}, [locations, props.hoverMarker, props.locFilt]);
Expand Down
Loading

0 comments on commit 8f9927a

Please sign in to comment.