diff --git a/src/App.js b/src/App.js
index 7d62f1a..ec088bc 100644
--- a/src/App.js
+++ b/src/App.js
@@ -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() {
@@ -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(
@@ -123,7 +135,6 @@ function App() {
// code just operates on the list of events passed to it.
return (
- /*
*/ /* frm: original code */
setCurrZip(newZip)} updateRange={(newRange) => setCurrRange(newRange)} updateEventKind={(newEventKind) => setCurrEventKind(newEventKind)} events={filteredEvents} updatedHover={(newHover) => setHoverEvent(newHover)} locFilt={locFilt} deviceIsMobile={deviceIsMobile}/>
{events === null && currZip == null &&
diff --git a/src/App.scss b/src/App.scss
index 1787a3c..d3719ea 100644
--- a/src/App.scss
+++ b/src/App.scss
@@ -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
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
{listEvents}
diff --git a/src/Map.js b/src/Map.js
index cae0a7f..8f3e0c3 100644
--- a/src/Map.js
+++ b/src/Map.js
@@ -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]);
diff --git a/src/MobileList.js b/src/MobileList.js
index 67d8b24..d51d4df 100644
--- a/src/MobileList.js
+++ b/src/MobileList.js
@@ -102,6 +102,13 @@ export function MobileList(props){
})
return (
+ /*
+ * frm: Original code that made the event text be an anchor tag.
+ *
+ * I changed this because with the new layout, the next and previous buttons
+ * are right next to the event text, making it too easy to mistakenly
+ * activate the anchor instead of just going to next/previous event.
+ *
{event['location']['venue']} in {event['location']['locality']}
+
+
Click to RSVP
+
+
+
)
}).filter((arrItem) => {
@@ -145,7 +168,7 @@ export function MobileList(props){
props.cardIndex > 0 &&
}
-
+
{
props.cardIndex < listEvents.length-1 &&
diff --git a/src/SearchBar.js b/src/SearchBar.js
index d46b7df..8d4fc24 100644
--- a/src/SearchBar.js
+++ b/src/SearchBar.js
@@ -43,7 +43,9 @@ export function SearchBar(props){
event.preventDefault();
props.updateRange(rangeInput); // calls setCurrRange() in App.js triggering a Mobilize API call and a re-render
setZip(input);
- // ??? frm: Should I add a call to setEventKind() here?
+ /* ??? frm: Should I add a call to setEventKind() here?
+ * I don't think it necessary until I put the event kind in the URL...
+ */
}
function setRange(input){
@@ -104,7 +106,6 @@ export function SearchBar(props){
*/
return(
- /*
Click to RSVP