pawarprasad123 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3824700492
##########
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:
relationshipSearch default is wrong (bug)
Global.ts sets relationshipSearch as a boolean (false default), but
SideBarBody.tsx:115 and CustomFiltersTree.tsx:46 use:
const { relationshipSearch = {} } = globalSessionData || {};
// ...
isVisible: !!relationshipSearch
An empty object {} is truthy → Relationships module shows even when feature
is disabled.
----
verify this and update if needed in both the files, also if changed, update
the test case.
##########
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:
Performance concern
expandedItemsMemo (lines 393–405) auto-expands every node on each keystroke
— may be slow on large entity trees.
verify this
All 6 trees stay mounted when collapsed
SideBarBody.tsx uses display: "none" instead of conditional render —
unnecessary DOM/memory when sidebar is collapsed.
##########
dashboard/src/styles/sidebar.scss:
##########
@@ -49,10 +49,10 @@
flex-grow: 1;
// position: fixed;
Review Comment:
#034858, #2ccebb, #4a90e2 appear in both TSX inline styles and sidebar.scss
instead of shared variables.
Lines 23, 48–52, 71–75
Commented dead code should be removed
.sidebar-searchbar padding conflicts with MUI sx in component, Lines 216–221
vs SidebarSearchInput.tsx
.sidebar-tree-label-nowrap added but tree uses inline whiteSpace: "nowrap"-
Unused class
Hardcoded hex colors instead of variables partial- Throughout
##########
dashboard/src/views/SideBar/SideBarTree/__tests__/SideBarTree.test.tsx:
##########
@@ -27,6 +27,7 @@
*/
Review Comment:
lines 1091–1110
Tests only assert tree renders — don't verify tooltip enable/disable (weaker
than lines 2220+) — consider removing duplicate weak tests
##########
dashboard/src/components/__tests__/SidebarSearchInput.test.tsx:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import React from "react";
+import { render, screen, fireEvent } from "@testing-library/react";
+import { SidebarSearchInput } from "../SidebarSearchInput";
+
+describe("SidebarSearchInput Component", () => {
Review Comment:
missing test
- Keyboard-only clear (Enter/Space on clear button), aria-label verification
##########
dashboard/src/views/SideBar/__tests__/SideBarBody.test.tsx:
##########
Review Comment:
missing test
- Version footer: loading spinner, V x.x display, 'Version unavailable'
error state
- relationshipSearch: false → Relationships module hidden (negative)
- Popover should NOT open when sidebar is expanded
- Only one popover open at a time when switching modules
-----
line 145: mocks relationshipSearch: {} which always enables Relationships —
this hides the production bug.
##########
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:
Search filter doesn't filter children (bug)
SideBarTree.tsx:355–365 keeps a parent if any child matches, but
renderTreeItem at line 1041 renders all children unfiltered. Users will see
non-matching siblings when searching.
--
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]
