Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3827430163


##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -101,61 +101,116 @@ const DrawerHeader = styled("div")(({ theme }) => ({
   marginBottom: "1rem",
 }));
 
+
 const SideBarBody = (props: {
-  loading: boolean;
-  handleOpenModal: any;
-  handleOpenAboutModal: any;
+  handleOpenModal: () => void;
+  handleOpenAboutModal: () => void;
 }) => {
   const location = useLocation();
   const routes = useRoutes(AppRoutes as RouteObject[]);
   const history = useHistory();
   const dispatch = useAppDispatch();
-  const { loading: loader, handleOpenModal, handleOpenAboutModal } = props;
+  const { handleOpenModal, handleOpenAboutModal } = props;
   const navigate = useNavigate();
-  const { loading } = useSelector((state: TypeHeaderState) => 
state.typeHeader);
   const { relationshipSearch = {} } = globalSessionData || {};

Review Comment:
   Fixed. Updated the default fallback from `{}` to `false` in both 
`SideBarBody.tsx` and `CustomFiltersTree.tsx` to respect the boolean schema 
from `Global.ts`. The test mocks in `SideBarBody.test.tsx` were also updated to 
use an explicit `true` boolean to properly maintain test coverage.
   



##########
dashboard/src/views/SideBar/SideBarTree/SideBarTree.tsx:
##########
@@ -287,1027 +322,1066 @@ const BarTreeView: FC<{
   sideBarOpen,
   searchTerm,
   loader,
+  isPopover,
 }) => {
-  const dispatch = useAppDispatch();
-  const { savedSearchData }: any = useAppSelector(
-    (state: any) => state.savedSearch
-  );
-  const { bmguid } = useParams();
-  const location = useLocation();
-  const navigate = useNavigate();
-  const searchParams = new URLSearchParams(location.search);
-  const [expand, setExpand] = useState<null | HTMLElement>(null);
-  const [selectedNode, setSelectedNode] = useState<{
-    type: string | null;
-    tag: string | null;
-    relationship: string | null;
-    businessMetadata: string | null;
-  }>({
-    type: null,
-    tag: null,
-    relationship: null,
-    businessMetadata: null,
-  });
-
-  const [openModal, setOpenModal] = useState<boolean>(false);
-  const toastId: any = useRef(null);
-  const open = Boolean(expand);
-  const [expandedItems, setExpandedItems] = useState<string[]>([]);
-  const [tagModal, setTagModal] = useState<boolean>(false);
-  const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
-  const { businessMetaData }: any = useAppSelector(
-    (state: any) => state.businessMetaData
-  );
-
-  const filteredData = useMemo(() => {
-    return treeData.filter((node) => {
-      return (
-        node.label?.toLowerCase().includes(searchTerm.toLowerCase()) ||
-        (node.children &&
-          node.children.some((child) =>
-            child.label?.toLowerCase().includes(searchTerm.toLowerCase())
-          ))
-      );
+    const { savedSearchData }: any = useAppSelector(
+      (state: any) => state.savedSearch
+    );
+    const { bmguid } = useParams();
+    const dispatch = useAppDispatch();
+    const location = useLocation();
+    const navigate = useNavigate();
+    const searchParams = new URLSearchParams(location.search);
+    const [expand, setExpand] = useState<null | HTMLElement>(null);
+    const [selectedNode, setSelectedNode] = useState<SelectedNode>({
+      type: null,
+      tag: null,
+      relationship: null,
+      businessMetadata: null,
+      term: null,
+      customFilter: null,
     });
-  }, [treeData, searchTerm]);
 
-  const displayTreeName = useMemo(() => {
-    return treeName === "CustomFilters" ? "Custom Filters" : treeName
-  }, [treeName]);
+    const [openModal, setOpenModal] = useState<boolean>(false);
+    const toastId: any = useRef(null);
+    const open = Boolean(expand);
+    const [expandedItems, setExpandedItems] = useState<string[]>([]);
+    const [tagModal, setTagModal] = useState<boolean>(false);
+    const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
+    const { businessMetaData }: any = useAppSelector(
+      (state: any) => state.businessMetaData
+    );
 
-  const highlightText = useMemo(() => {
-    return (text: string) => {
-      if (!searchTerm) return text;
+    const filteredData = useMemo(() => {

Review Comment:
   Fixed. Updated the search filtering logic in `SideBarTree.tsx`. Now, if a 
child matches but its parent doesn't, we explicitly filter the parent's 
`children` array to only retain the matching children, preventing non-matching 
siblings from being incorrectly rendered in the UI. All test cases have been 
verified to pass.
   



##########
dashboard/src/views/SideBar/SideBarTree/SideBarTree.tsx:
##########
@@ -287,1027 +322,1066 @@ const BarTreeView: FC<{
   sideBarOpen,
   searchTerm,
   loader,
+  isPopover,
 }) => {
-  const dispatch = useAppDispatch();
-  const { savedSearchData }: any = useAppSelector(
-    (state: any) => state.savedSearch
-  );
-  const { bmguid } = useParams();
-  const location = useLocation();
-  const navigate = useNavigate();
-  const searchParams = new URLSearchParams(location.search);
-  const [expand, setExpand] = useState<null | HTMLElement>(null);
-  const [selectedNode, setSelectedNode] = useState<{
-    type: string | null;
-    tag: string | null;
-    relationship: string | null;
-    businessMetadata: string | null;
-  }>({
-    type: null,
-    tag: null,
-    relationship: null,
-    businessMetadata: null,
-  });
-
-  const [openModal, setOpenModal] = useState<boolean>(false);
-  const toastId: any = useRef(null);
-  const open = Boolean(expand);
-  const [expandedItems, setExpandedItems] = useState<string[]>([]);
-  const [tagModal, setTagModal] = useState<boolean>(false);
-  const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
-  const { businessMetaData }: any = useAppSelector(
-    (state: any) => state.businessMetaData
-  );
-
-  const filteredData = useMemo(() => {
-    return treeData.filter((node) => {
-      return (
-        node.label?.toLowerCase().includes(searchTerm.toLowerCase()) ||
-        (node.children &&
-          node.children.some((child) =>
-            child.label?.toLowerCase().includes(searchTerm.toLowerCase())
-          ))
-      );
+    const { savedSearchData }: any = useAppSelector(
+      (state: any) => state.savedSearch
+    );
+    const { bmguid } = useParams();
+    const dispatch = useAppDispatch();
+    const location = useLocation();
+    const navigate = useNavigate();
+    const searchParams = new URLSearchParams(location.search);
+    const [expand, setExpand] = useState<null | HTMLElement>(null);
+    const [selectedNode, setSelectedNode] = useState<SelectedNode>({
+      type: null,
+      tag: null,
+      relationship: null,
+      businessMetadata: null,
+      term: null,
+      customFilter: null,
     });
-  }, [treeData, searchTerm]);
 
-  const displayTreeName = useMemo(() => {
-    return treeName === "CustomFilters" ? "Custom Filters" : treeName
-  }, [treeName]);
+    const [openModal, setOpenModal] = useState<boolean>(false);
+    const toastId: any = useRef(null);
+    const open = Boolean(expand);
+    const [expandedItems, setExpandedItems] = useState<string[]>([]);
+    const [tagModal, setTagModal] = useState<boolean>(false);
+    const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
+    const { businessMetaData }: any = useAppSelector(
+      (state: any) => state.businessMetaData
+    );
 
-  const highlightText = useMemo(() => {
-    return (text: string) => {
-      if (!searchTerm) return text;
+    const filteredData = useMemo(() => {
+      return treeData.filter((node) => {
+        return (
+          node.label?.toLowerCase().includes(searchTerm.toLowerCase()) ||
+          (node.children &&
+            node.children.some((child) =>
+              child.label?.toLowerCase().includes(searchTerm.toLowerCase())
+            ))
+        );
+      });
+    }, [treeData, searchTerm]);
 
-      const parts = text.split(new RegExp(`(${searchTerm})`, "gi"));
-      return parts.map((part, index) =>
-        part.toLowerCase() === searchTerm.toLowerCase() ? (
-          <span key={index} style={{ color: "#D3D3D3", fontWeight: "600" }}>
-            {part}
-          </span>
-        ) : (
-          part
-        )
-      );
-    };
-  }, [searchTerm]);
-
-  const expandedItemsMemo = useMemo(() => {
-    const allNodeIds = filteredData.flatMap((node) => {
-      return [
-        node.id,
-        ...(node.children ? node.children.map((child) => child.id) : []),
-      ];
-    });
-    return [...allNodeIds, ...[treeName]];
-  }, [filteredData, treeName]);
+    const displayTreeName = useMemo(() => {
+      return treeName === "CustomFilters" ? "Custom Filters" : treeName
+    }, [treeName]);
 
-  useEffect(() => {
-    setExpandedItems(expandedItemsMemo);
-  }, [expandedItemsMemo]);
+    const highlightText = useMemo(() => {
+      return (text: string) => {
+        if (!searchTerm) return text;
 
-  useEffect(() => {
-    const searchParams = new URLSearchParams(location.search);
-    const nodeIdFromParamsType = searchParams.get("type");
-    const nodeIdFromParamsTag = searchParams.get("tag");
-    const nodeIdFromParamsRelationshipName =
-      searchParams.get("relationshipName");
-    const nodeIdFromBMName = location.pathname.includes(
-      "/administrator/businessMetadata"
-    );
+        const escapeRegExp = (string: string) => {
+          return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means 
the whole matched string
+        };
+
+        const escapedSearchTerm = escapeRegExp(searchTerm);
+        const parts = text.split(new RegExp(`(${escapedSearchTerm})`, "gi"));
+        return parts.map((part, index) =>
+          part.toLowerCase() === searchTerm.toLowerCase() ? (
+            <span key={index} className="sidebar-tree-highlight">
+              {part}
+            </span>
+          ) : (
+            part
+          )
+        );
+      };
+    }, [searchTerm]);
+
+    const expandedItemsMemo = useMemo(() => {

Review Comment:
   Fixed. 
   1. **Performance:** `expandedItemsMemo` has been updated so it no longer 
auto-expands the entire tree when `searchTerm` is empty. It now correctly falls 
back to expanding only the root node. When a search matches, it also 
specifically omits leaf node IDs from the expanded items list, preventing 
unnecessary heavy processing.
   2. **DOM / Memory:** Refactored `SideBarBody.tsx` to conditionally render 
the entire `<Paper>` wrapper with `{open && (...) }` instead of using `display: 
"none"`. The 6 trees now properly unmount from the DOM whenever the sidebar is 
collapsed.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to