From 34ad4da6b2d4461734855786063d0a55b48374ae Mon Sep 17 00:00:00 2001
From: JEFFREY-Bonson <jeffrey.bonson@ibm.com>
Date: Fri, 1 Mar 2024 18:26:04 +0530
Subject: [PATCH] fixing the review comments

---
 .../AeInlineMethod/FilterNamespace.jsx        | 10 +--
 .../AeInlineMethod/NamespaceSelector.jsx      | 10 +--
 .../components/AeInlineMethod/index.jsx       | 62 +++++++------------
 .../components/AeInlineMethod/style.scss      |  4 ++
 4 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/app/javascript/components/AeInlineMethod/FilterNamespace.jsx b/app/javascript/components/AeInlineMethod/FilterNamespace.jsx
index cbff2ef1cdda..1d79f0405b39 100644
--- a/app/javascript/components/AeInlineMethod/FilterNamespace.jsx
+++ b/app/javascript/components/AeInlineMethod/FilterNamespace.jsx
@@ -5,7 +5,7 @@ import {
 } from 'carbon-components-react';
 import { noSelect } from './helper';
 
-const FilterNamespace = ({ domains, filterOnChange }) => {
+const FilterNamespace = ({ domains, onSearch }) => {
   /** Function to render the search text. */
   const renderSearchText = () => (
     <div className="search-wrapper">
@@ -14,8 +14,8 @@ const FilterNamespace = ({ domains, filterOnChange }) => {
         id="search-method"
         labelText={__('Search')}
         placeholder={__('Search with Name or Relative path')}
-        onClear={() => filterOnChange('')}
-        onChange={(event) => filterOnChange(event.target.value || noSelect)}
+        onClear={() => onSearch('')}
+        onChange={(event) => onSearch({ searchText: event.target.value || noSelect })}
       />
     </div>
   );
@@ -27,7 +27,7 @@ const FilterNamespace = ({ domains, filterOnChange }) => {
       labelText="Select a domain"
       defaultValue="option"
       size="lg"
-      onChange={(event) => filterOnChange({ selectedDomain: event.target.value })}
+      onChange={(event) => onSearch({ selectedDomain: event.target.value })}
     >
       <SelectItem value={noSelect} text="None" />
       {
@@ -48,7 +48,7 @@ export default FilterNamespace;
 
 FilterNamespace.propTypes = {
   domains: PropTypes.arrayOf(PropTypes.any),
-  filterOnChange: PropTypes.func.isRequired,
+  onSearch: PropTypes.func.isRequired,
 };
 
 FilterNamespace.defaultProps = {
diff --git a/app/javascript/components/AeInlineMethod/NamespaceSelector.jsx b/app/javascript/components/AeInlineMethod/NamespaceSelector.jsx
index fc6e76368ceb..be5f5aa362ba 100644
--- a/app/javascript/components/AeInlineMethod/NamespaceSelector.jsx
+++ b/app/javascript/components/AeInlineMethod/NamespaceSelector.jsx
@@ -32,9 +32,9 @@ const NamespaceSelector = ({ onSelectMethod, selectedIds }) => {
   }, []);
 
   /** Function to handle search text and drop-down item onchange events. */
-  const handleFilterOnChange = (filterData) => {
+  const onSearch = (filterData) => {
     updateData({ loading: true });
-    const searchText = filterData.searchText ? filterData.text : data.searchText;
+    const searchText = filterData.searchText ? filterData.searchText : data.searchText;
     const selectedDomain = filterData.selectedDomain ? filterData.selectedDomain : data.selectedDomain;
     const url = searchUrl(selectedDomain, searchText);
     http.get(url)
@@ -46,7 +46,7 @@ const NamespaceSelector = ({ onSelectMethod, selectedIds }) => {
   };
 
   /** Function to handle the click events for the list. */
-  const onCellClickHandler = (selectedRow, cellType, checked) => {
+  const onCellClick = (selectedRow, cellType, checked) => {
     const selectedItems = cellType === CellAction.selectAll
       ? data.methods.map((item) => item.id)
       : [selectedRow];
@@ -64,14 +64,14 @@ const NamespaceSelector = ({ onSelectMethod, selectedIds }) => {
         rowCheckBox
         sortable={false}
         gridChecks={selectedIds}
-        onCellClick={(selectedRow, cellType, event) => onCellClickHandler(selectedRow, cellType, event.target.checked)}
+        onCellClick={(selectedRow, cellType, event) => onCellClick(selectedRow, cellType, event.target.checked)}
       />
     )
     : <NotificationMessage type="error" message={__('No methods available.')} />);
 
   return (
     <div className="inline-method-selector">
-      <FilterNamespace domains={data.domains} filterOnChange={(filterData) => handleFilterOnChange(filterData)} />
+      <FilterNamespace domains={data.domains} onSearch={(filterData) => onSearch(filterData)} />
       <div className="inline-contents-wrapper">
         {
           data.loading
diff --git a/app/javascript/components/AeInlineMethod/index.jsx b/app/javascript/components/AeInlineMethod/index.jsx
index d4ab598ab2f9..adabaf247d31 100644
--- a/app/javascript/components/AeInlineMethod/index.jsx
+++ b/app/javascript/components/AeInlineMethod/index.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import {
   Modal, Button, ModalBody, Accordion, AccordionItem,
@@ -18,42 +18,21 @@ const AeInlineMethod = ({ type }) => {
     rows: [],
   });
 
-  useEffect(() => {
-    if (!data.isModalOpen) {
-      if (data.selectedIds.length > 0) {
-        http.get(`${namespaceUrls.aeMethodsUrl}?ids=${data.selectedIds.map((str) => parseInt(str, 10))}`)
-          .then(({ methods }) => {
-            setData({ ...data, rows: formatListMethods(methods) });
-          });
-      } else {
-        setData({ ...data, rows: [] });
-      }
-    }
-  }, [data.isModalOpen]);
-
   /** Function to show/hide the modal. */
   const showModal = (status) => setData({ ...data, isModalOpen: status });
 
   /** Function to handle the select-all check-box click event. */
-  const onSelectAll = (selectedItems, checked) => {
-    setData({
-      ...data,
-      selectedIds: checked ? [...selectedItems] : [],
-    });
-  };
+  const onSelectAll = (selectedItems, checked) => setData({ ...data, selectedIds: checked ? [...selectedItems] : [] });
 
   /** Function to handle the list row selection events.
    * selectedItem is passed as an array. */
-  const onItemSelect = (selectedItem, checked) => {
+  const onItemSelect = (selectedItems, checked) => {
     if (checked) {
-      data.selectedIds.push(selectedItem[0].id);
+      data.selectedIds.push(selectedItems[0].id);
     } else {
-      data.selectedIds = data.selectedIds.filter((item) => item !== selectedItem[0].id);
+      data.selectedIds = data.selectedIds.filter((item) => item !== selectedItems[0].id);
     }
-    setData({
-      ...data,
-      selectedIds: [...data.selectedIds],
-    });
+    setData({ ...data, selectedIds: [...data.selectedIds] });
   };
 
   /** Function to add/remove an selected items. */
@@ -66,13 +45,21 @@ const AeInlineMethod = ({ type }) => {
 
   /** Function to handle the click events for the list. */
   const onCellClickHandler = (item) => {
-    if (item && item.callbackAction) {
-      if (item.callbackAction === 'removeMethod') {
-        setData({
-          rows: data.rows.filter((row) => row.id !== item.id),
-          selectedIds: data.selectedIds.filter((id) => id !== item.id),
-        });
-      }
+    if (item && item.callbackAction && item.callbackAction === 'removeMethod') {
+      setData({
+        rows: data.rows.filter((row) => row.id !== item.id),
+        selectedIds: data.selectedIds.filter((id) => id !== item.id),
+      });
+    }
+  };
+
+  /** Function to handle the modal submit action. */
+  const submitModal = () => {
+    if (data.selectedIds.length > 0) {
+      http.get(`${namespaceUrls.aeMethodsUrl}?ids=${data.selectedIds.map((str) => parseInt(str, 10))}`)
+        .then(({ methods }) => setData({ ...data, rows: formatListMethods(methods), isModalOpen: false }));
+    } else {
+      setData({ ...data, rows: [], isModalOpen: false });
     }
   };
 
@@ -86,7 +73,7 @@ const AeInlineMethod = ({ type }) => {
       primaryButtonText={__('OK')}
       secondaryButtonText={__('Cancel')}
       onRequestClose={() => showModal(false)}
-      onRequestSubmit={() => showModal(false)}
+      onRequestSubmit={() => submitModal()}
       onSecondarySubmit={() => showModal(false)}
     >
       <ModalBody>
@@ -115,10 +102,9 @@ const AeInlineMethod = ({ type }) => {
       />
     )
     : (
-      <>
-        <br />
+      <div className="ae-inline-methods-notification">
         <NotificationMessage type="info" message={__('No methods selected.')} />
-      </>
+      </div>
     ));
 
   const renderAddButton = () => (
diff --git a/app/javascript/components/AeInlineMethod/style.scss b/app/javascript/components/AeInlineMethod/style.scss
index d9b0808650fa..3b34f7fec309 100644
--- a/app/javascript/components/AeInlineMethod/style.scss
+++ b/app/javascript/components/AeInlineMethod/style.scss
@@ -66,5 +66,9 @@
       display: flex;
       justify-content: flex-end;
     }
+
+    .ae-inline-methods-notification {
+      margin-top: 20px;
+    }
   }
 }