Merge pull request #3 from techforwarren/master

Merge out from techforwarren to my fork's master

src/App.js

@@ -94,19 +94,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(
@@ -122,18 +134,18 @@ function App() {
   //      code just operates on the list of events passed to it.
 
   return (
-    <div className="app">
+    <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 &&
         <div id="startLoad">
           <h1 id="firstLine">SHE HAS</h1><h1 id="secondLine">EVENTS</h1><h1 id="thirdLine">FOR THAT <img src={gMark} alt=""></img></h1>
           <h3 id="searchCTA">Enter your zipcode to find events near you!</h3>
         </div>
       }
+      <Map currZip={currZip} events={filteredEvents} hoverMarker={hoverEvent} selectLoc={(newLoc) => setLocFilt(newLoc)} locFilt={locFilt}/>
       {filteredEvents !== null && deviceIsMobile &&
         <MobileList events={filteredEvents} updatedHover={(newHover) => setHoverEvent(newHover)} locFilt={locFilt} cardIndex={cardIndex} updateCardIndex={(update) => setCardIndex(update)}/>
       }
-      <Map currZip={currZip} events={filteredEvents} hoverMarker={hoverEvent} selectLoc={(newLoc) => setLocFilt(newLoc)} locFilt={locFilt}/>
     </div>
   );
 }

src/App.scss

@@ -1,9 +1,52 @@
-.app{
+/* 
+ *
+ * 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;    /* on desktop use a two column grid layout - right column grows/shrinks */
+  grid-template-columns: minmax(180px,300px) auto;
+
   #map{
-    height: 100%;
+    height: 100vh;
     z-index: 9;
   }
 
+}
+
+.appIsMobile {
+  display: grid;    /* on mobile use a row grid with three rows: search, map, card & nav */
+  grid-template-rows: min-content auto min-content;
+
+  #map{
+    z-index: 9;                 /* frm: not sure z-index does anything for mobile */
+  }
+
+}
+
+.app{
+  height: 100vh;                
+
   #startLoad{
     position: fixed;
     width: 100%;
@@ -39,7 +82,7 @@
           height: 10vw;
         }
       }
-    }
+    } 
     #secondLine{
       font-size: 20vh;
       color: white;
@@ -70,14 +113,16 @@ html, body, #root, .app {
 
 .searchBar{
   background-color: #232444;
-  //width: 300px;
-  width: 32%;
-  position: fixed;
-  left: 2%;
-  top: 2%;
-  z-index:11;
-  box-shadow: 10px 10px 5px rgba(0, 0, 0, .3);
+  z-index:11;                   /* frm: not sure we need z-index anymore */
+  box-shadow: 10px 0px 5px rgba(0, 0, 0, .3);   
+}
 
+.desktopSearch {
+  /* 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 {
@@ -120,7 +165,7 @@ html, body, #root, .app {
   align-items: flex-end;
   justify-content: center;
   padding-left: 5%;
-  padding-right: 7%;
+  padding-right: 5%;    
   padding-top: 5%;
   padding-bottom: 5%;
 
@@ -221,7 +266,7 @@ html, body, #root, .app {
   font-size: 16px;
   background-color: #f5f5f5;
   border: none;
-  padding: 0px 20px 0px 5px;
+  padding: 0px 5px 0px 5px;    
   appearance: none;
   -moz-appearance: none;
   -webkit-appearance: none;
@@ -269,7 +314,6 @@ html, body, #root, .app {
 
 .eventList{
   background-color: white;
-  height: 500px;
   overflow:hidden;
   overflow-y:scroll;
   margin-bottom: 0px;
@@ -330,25 +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;
+
   .eventCard{
 
     text-decoration: none;
     color: #232444;
-    position: fixed;
     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;
+    min-height: 10vh;  
 
     h3{
-      margin-top: 10px;  /* frm: smaller for mobile to conserve whitespace */
+      margin-top: 10px;  
       margin-bottom: 10px;
       text-transform: uppercase;
     }
@@ -358,44 +399,47 @@ html, body, #root, .app {
     }
 
     .mobileInfo{
-
       padding-left: 5%;
       padding-right: 5%;
-      padding-bottom: 2%;  /* frm: added this to make sure we had some whitespace at bottom (after reducing min-height of .eventCard */
+      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: fixed;
-    bottom: 3%;         /* frm: changed from 5% to 3% */
+    bottom: 3%;         
     z-index: 10;
     background-color: #232444;
     border: none;
     color: white;
-    font-size: 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%;
-    width: 30vw;
+    flex: 1;            /* Info/RSVP button stretches to use up all available space */
     a{
       text-decoration: none;
       color:white;
     }
   }
   #leftIndex{
-    left: 25px;
+    width: 50px;
+    border-right: 3px solid white;      /* separate nav button from RSVP/Info button */
   }
   #rightIndex{
-    right: 25px;
+    width: 50px;
+    border-left: 3px solid white;       /* separate nav button from RSVP/Info button */
   }
 }
 

src/EventList.js

@@ -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){
@@ -77,7 +66,7 @@ export function EventList(props) {
       }
     })
 
-    //Location filter
+    //Location filter: if user has clicked on a marker, then only show that one event
     if(props.locFilt != null){
       if(eventHasValidLocation(event)) {
         if(event['location']['location']['latitude'] !== props.locFilt['lat'] || event['location']['location']['longitude'] !== props.locFilt['lng']){
@@ -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}

src/Map.js

@@ -39,7 +39,7 @@ export function Map(props){
               	zoom = map.getZoom(),
               	delta = this._delta,
               	normalizedDelta = 0,
-              	snap = this._map.options.zoomSnap || 0;
+              	snap = this._map.options.zoomSnap || 0;  // ??? frm: why use this._map instead of just map
 
     		  wheelDeltaList.push(Math.abs(delta));
     		  var average = 0;
@@ -59,7 +59,7 @@ export function Map(props){
 
     		  var deltaTime = currentScrollTime - lastScroll;
 
-    		  var d2 = this._delta / (this._map.options.wheelPxPerZoomLevel * 4),
+    		  var d2 = this._delta / (this._map.options.wheelPxPerZoomLevel * 4),  // ??? frm: why use this._map instead of just map
     		  d3 = 4 * Math.log(2 / (1 + Math.exp(-Math.abs(d2)))) / Math.LN2,
     		  d4 = snap ? Math.ceil(d3 / snap) * snap : d3,
                     normalizedDelta = map._limitZoom(zoom + (this._delta > 0 ? d4 : -d4)) - zoom;
@@ -159,8 +159,31 @@ export function Map(props){
 
   		}
 
-      // zoom to marker bounds, plus padding (percentage)
-      map.current.fitBounds(markers.current.getBounds().pad(0.5));
+      /* ??? 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]);
 
@@ -196,7 +219,14 @@ export function Map(props){
             const lat = props.events[first]['location']['location']['latitude'];
             const long = props.events[first]['location']['location']['longitude'];
 
-            if(center[0] !== lat || center[0] !== long){
+            /* 
+             * If "first" non-private location is different from previous "center"
+             * then update the "center".  This will center the map view on the 
+             * "first" non-private location which we assume is the one closest to
+             * the given zip code (the Mobilize API should return events in the 
+             * order of closest first)
+             */
+            if(center[0] !== lat || center[1] !== long){  
               setCenter([lat, long]);
               setNewCenter(true);
             }