From 00185c1b69f555ae3932fc7b754584aeb5dfec81 Mon Sep 17 00:00:00 2001
From: rogup <rohit5.gup@gmail.com>
Date: Wed, 1 May 2024 14:57:50 +0100
Subject: [PATCH] Small refactoring based on code review comments

---
 src/map-app/app/map-ui.ts                   |  6 ++---
 src/map-app/app/view/sidebar.ts             |  6 ++---
 src/map-app/app/view/sidebar/initiatives.ts | 26 ++++++++++-----------
 src/map-app/utils.ts                        | 15 ++++++++++++
 4 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/map-app/app/map-ui.ts b/src/map-app/app/map-ui.ts
index f8a24697..898f7a42 100644
--- a/src/map-app/app/map-ui.ts
+++ b/src/map-app/app/map-ui.ts
@@ -8,7 +8,7 @@ import { EventBus } from "../eventbus";
 import "./map"; // Seems to be needed to prod the leaflet CSS into loading.
 import { SidebarPresenter } from "./presenter/sidebar";
 import { PhraseBook } from "../localisations";
-import { toString as _toString, getViewportWidth } from '../utils';
+import { toString as _toString, canDisplayExpandedSidebar } from '../utils';
 import { Action, AppState, PropEquality, StateManager, TextSearch } from "./state-manager";
 import { StateChange } from "../undo-stack";
 import { Dictionary } from "../common-types";
@@ -138,7 +138,7 @@ export class MapUI {
       this.stateManager.propFilter(new PropEquality(propName, value, isMulti));
     }
 
-    if (getViewportWidth() > 1080) // on smaller screens, wait until user clicks Apply Filters
+    if (canDisplayExpandedSidebar()) // on smaller screens, wait until user clicks Apply Filters
       EventBus.Sidebar.showInitiativeList.pub();
   }
 
@@ -161,7 +161,7 @@ export class MapUI {
     console.log("Search submitted: [" + text + "]");
     this.stateManager.textSearch(new TextSearch(text));
 
-    if (getViewportWidth() > 1080) {// on smaller screens, wait until user clicks Apply Filters
+    if (canDisplayExpandedSidebar()) {// on smaller screens, wait until user clicks Apply Filters
       EventBus.Sidebar.showInitiativeList.pub();
     }
   }
diff --git a/src/map-app/app/view/sidebar.ts b/src/map-app/app/view/sidebar.ts
index b39af4f6..e83b95ec 100644
--- a/src/map-app/app/view/sidebar.ts
+++ b/src/map-app/app/view/sidebar.ts
@@ -3,7 +3,7 @@ import { EventBus } from '../../eventbus';
 import * as d3 from 'd3';
 import { SidebarPresenter } from '../presenter/sidebar';
 import { BaseView } from './base';
-import { getViewportWidth } from '../../utils';
+import { canDisplayInitiativePopups } from '../../utils';
 import { Initiative } from '../model/initiative';
 
 export class SidebarView extends BaseView {
@@ -121,8 +121,6 @@ export class SidebarView extends BaseView {
     d3.select("#map-app-sidebar i").attr("class", "fa fa-angle-left");
 
     this.presenter.changeSidebar(); // Refresh the content of the sidebar
-    if (document.getElementById("dir-filter") && getViewportWidth() > 1080)
-      document.getElementById("dir-filter")?.focus();
   }
 
   updateSidebarWidth() {
@@ -243,7 +241,7 @@ export class SidebarView extends BaseView {
       .html(initiativeContent);
     initiativeSidebar.classed("sea-initiative-sidebar-open", true);
 
-    if (getViewportWidth() <= 800)
+    if (!canDisplayInitiativePopups())
       EventBus.Sidebar.showSidebar.pub();
   }
 }
diff --git a/src/map-app/app/view/sidebar/initiatives.ts b/src/map-app/app/view/sidebar/initiatives.ts
index e74f4909..5bf2abdc 100644
--- a/src/map-app/app/view/sidebar/initiatives.ts
+++ b/src/map-app/app/view/sidebar/initiatives.ts
@@ -48,18 +48,18 @@ export class InitiativesSidebarView extends BaseSidebarView {
     d3.select("#search-box").property("value", txt);
   }
 
-  submitCallback = (event: Event) => {
-    event.preventDefault(); // prevent page reloads on submit
-
-    const oldSearchText = this.presenter.parent.mapui.getSearchText()
-    const newSearchText = this.getSearchText();
-
-    if (newSearchText !== oldSearchText) {
-      this.presenter.performSearch(newSearchText);
-    }
-  };
-
   createSearchBox(container: d3DivSelection) {
+    const submitCallback = (event: Event) => {
+      event.preventDefault(); // prevent page reloads on submit
+  
+      const oldSearchText = this.presenter.parent.mapui.getSearchText()
+      const newSearchText = this.getSearchText();
+  
+      if (newSearchText !== oldSearchText) {
+        this.presenter.performSearch(newSearchText);
+      }
+    };
+
     const form = container
       .append("form")
       .attr("id", "map-app-search-form")
@@ -67,7 +67,7 @@ export class InitiativesSidebarView extends BaseSidebarView {
         "class",
         "w3-card-2 w3-round map-app-search-form"
       )
-      .on("submit", this.submitCallback);
+      .on("submit", submitCallback);
     
     const selection = form
       .append("div")
@@ -95,7 +95,7 @@ export class InitiativesSidebarView extends BaseSidebarView {
       .attr("autocomplete", "off")
       // If we don't submit the search on blur, selecting another filter will reset the search text
       // https://github.com/digitalcommons/mykomap/issues/197
-      .on("blur", this.submitCallback);
+      .on("blur", submitCallback);
   }
 
   private createFilterCount(container: d3DivSelection) {
diff --git a/src/map-app/utils.ts b/src/map-app/utils.ts
index f35ff0f4..544a5c79 100644
--- a/src/map-app/utils.ts
+++ b/src/map-app/utils.ts
@@ -256,3 +256,18 @@ export function filterSet<T>(set: Set<T>, predicate: Predicate<T>): Set<T> {
  */
 export const getViewportWidth = () =>
   Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0);
+
+/**
+ * We can display an expanded sidebar if the viewport is wide enough (over 1080 pixels), otherwise
+ * we collapse to a single sidebar.
+ */
+export const canDisplayExpandedSidebar = () =>
+  getViewportWidth() > 1080;
+
+
+  /**
+ * We can display initiative popups if the viewport is wide enough (over 800 pixels), otherwise we
+ * display the info in the sidebar.
+ */
+export const canDisplayInitiativePopups = () =>
+  getViewportWidth() > 800;