Address PR comments

jameel-institute · Sep 10, 2024 · fbb2a05 · fbb2a05
1 parent 143063c
commit fbb2a05
Show file tree

Hide file tree

Showing 7 changed files with 9 additions and 71 deletions.
diff --git a/components/ParameterForm.vue b/components/ParameterForm.vue
@@ -4,7 +4,6 @@
       v-if="props.metadata && formData"
       class="inputs"
       role="form"
-      :data-test-form-data="JSON.stringify(formData)"
       :data-test-navigate-to="navigateToData"
       @submit.prevent="submitForm"
     >
@@ -24,7 +23,7 @@
             <CButtonGroup
               role="group"
               :aria-label="parameter.label"
-              :size="screenIsLarge ? 'lg' : undefined"
+              :size="appStore.largeScreen ? 'lg' : undefined"
             >
               <!-- This component's "v-model" prop type signature dictates we can't pass it a number. -->
               <CFormCheck
@@ -51,7 +50,7 @@
             :id="parameter.id"
             v-model="formData[parameter.id]"
             :aria-label="parameter.label"
-            class="form-select" :class="[screenIsLarge ? 'form-select-lg' : '']"
+            class="form-select" :class="[appStore.largeScreen ? 'form-select-lg' : '']"
           >
             <option
               v-for="(option) in parameter.options"
@@ -67,7 +66,7 @@
       <CButton
         id="run-button"
         color="primary"
-        :size="screenIsLarge ? 'lg' : undefined"
+        :size="appStore.largeScreen ? 'lg' : undefined"
         type="submit"
         :disabled="formSubmitting"
         @click="submitForm"
@@ -109,7 +108,6 @@ const formData = ref(
 );
 
 const appStore = useAppStore();
-const { screenIsLarge } = storeToRefs(appStore);
 const navigateToData = ref("");
 const pageMounted = ref(false);
 

diff --git a/components/SideBar.vue b/components/SideBar.vue
@@ -1,12 +1,12 @@
 <template>
   <CSidebar
     :visible="visible"
-    :unfoldable="screenIsLarge"
-    :overlaid="screenIsLarge"
+    :unfoldable="appStore.largeScreen"
+    :overlaid="appStore.largeScreen"
     class="sidebar-fixed border-end"
     @hide="handleHide"
   >
-    <CSidebarNav role="navigation" data-testid="sidebar">
+    <CSidebarNav role="navigation">
       <CNavItem>
         <NuxtLink prefetch-on="interaction" to="/scenarios/new" class="nav-link">
           <CIcon icon="cilPlus" size="lg" class="nav-icon" /> New scenario
@@ -72,10 +72,9 @@ const versionTooltipContent = computed(() => {
 const visible = defineModel("visible", { type: Boolean, required: true });
 
 const appStore = useAppStore();
-const { screenIsLarge } = storeToRefs(appStore);
 
 const resetSidebarPerScreenSize = () => {
-  visible.value = screenIsLarge.value;
+  visible.value = appStore.largeScreen;
 };
 
 const hideHasNeverBeenEmitted = ref(true);

diff --git a/layouts/default.vue b/layouts/default.vue
@@ -25,16 +25,15 @@ function handleToggleSidebarVisibility() {
 }
 
 const appStore = useAppStore(useNuxtApp().$pinia);
-appStore.initializeAppState();
 
 const setScreenSize = () => {
   // As this function uses window, it can only be run client-side, so we have to pass the $pinia instance
   // to the useAppStore function: https://pinia.vuejs.org/ssr/nuxt.html#Awaiting-for-actions-in-pages
   const breakpoint = 992; // CoreUI's "lg" breakpoint
   if (window.innerWidth < breakpoint) {
-    appStore.setScreenSize(false);
+    appStore.largeScreen = false;
   } else {
-    appStore.setScreenSize(true);
+    appStore.largeScreen = true;
   }
 };
 

diff --git a/stores/appStore.ts b/stores/appStore.ts
@@ -5,17 +5,4 @@ export const useAppStore = defineStore("app", {
   state: (): AppState => ({
     largeScreen: true,
   }),
-  getters: {
-    screenIsLarge: (state: AppState): boolean => {
-      return state.largeScreen;
-    },
-  },
-  actions: {
-    initializeAppState(): void {
-      this.largeScreen = true;
-    },
-    setScreenSize(large: boolean): void {
-      this.largeScreen = large;
-    },
-  },
 });
diff --git a/tests/e2e/runAnalysis.spec.ts b/tests/e2e/runAnalysis.spec.ts
@@ -17,8 +17,6 @@ test("Can request a scenario analysis run", async ({ page, baseURL }) => {
   await page.selectOption('select[id="country"]', { label: "United States" });
   await page.click('div[aria-label="Global vaccine investment"] label[for="low"]');
 
-  await expect(page.getByRole("form")).toHaveAttribute("data-test-form-data", "{\"country\":\"United States\",\"pathogen\":\"influenza-1957\",\"response\":\"elimination\",\"vaccine\":\"low\"}");
-
   await page.click('button:has-text("Run")');
 
   const dummyRunIdHash = "007e5f5453d64850";

diff --git a/tests/unit/components/ParameterForm.spec.ts b/tests/unit/components/ParameterForm.spec.ts
@@ -106,39 +106,6 @@ describe("parameter form", () => {
     ]);
   });
 
-  it("initialises formData with defaults and updates formData when a parameter is changed", async () => {
-    const component = await mountSuspended(ParameterForm, {
-      props: { metadata, metadataFetchStatus: "success", metadataFetchError: null },
-      global: { stubs },
-    });
-
-    const cForm = component.findComponent({ name: "CForm" });
-    let formData = JSON.parse(cForm.element.attributes.getNamedItem("data-test-form-data")!.value);
-    expect(formData.region).toBe("HVN");
-    expect(formData.long_list).toBe("1");
-    expect(formData.short_list).toBe("no");
-
-    const selectElements = component.findAll("select");
-    const longListDropDown = selectElements[0];
-    const countrySelect = selectElements[1];
-
-    // Verify that the select elements are the ones we think they are
-    selectElements.forEach((selectElement, index) => {
-      const correctLabel = ["Drop Down", "Region"][index];
-      const paramId = selectElement.element.attributes.getNamedItem("id")!.value;
-      expect(component.find(`label[for=${paramId}]`).element.textContent).toBe(correctLabel);
-    });
-
-    await longListDropDown.findAll("option").at(2)!.setSelected();
-    await countrySelect.findAll("option").at(0)!.setSelected();
-    await component.findComponent({ name: "CButtonGroup" }).find("input[value='yes']").setChecked();
-
-    formData = JSON.parse(cForm.element.attributes.getNamedItem("data-test-form-data")!.value);
-    expect(formData.region).toBe("CLD");
-    expect(formData.long_list).toBe("3");
-    expect(formData.short_list).toBe("yes");
-  });
-
   it("sends a POST request to /api/scenarios with the form data when submitted", async () => {
     registerEndpoint("/api/scenarios", {
       method: "POST",

diff --git a/tests/unit/stores/appStore.spec.ts b/tests/unit/stores/appStore.spec.ts
@@ -11,16 +11,6 @@ describe("app store", () => {
     it("initialises correctly", async () => {
       const store = useAppStore();
       expect(store.largeScreen).toBe(true);
-      store.initializeAppState();
-      expect(store.largeScreen).toBe(true);
-    });
-
-    it("can update and retrieve the screen size", async () => {
-      const store = useAppStore();
-      const { screenIsLarge } = storeToRefs(store);
-
-      store.setScreenSize(false);
-      expect(screenIsLarge.value).toBe(false);
     });
   });
 });