Brijesh619 commented on code in PR #688: URL: https://github.com/apache/atlas/pull/688#discussion_r3860042350
########## dashboard/src/components/SidebarSearchInput.tsx: ########## @@ -0,0 +1,76 @@ +/* + * 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, { ChangeEvent } from "react"; +import { Paper, InputBase, Stack } from "@mui/material"; +import ClearIcon from "@mui/icons-material/Clear"; +import { IconButton } from "@components/muiComponents"; + +interface SidebarSearchInputProps { + searchTerm: string; + onChange: (value: string) => void; + dataCy?: string; +} + +export const SidebarSearchInput: React.FC<SidebarSearchInputProps> = ({ + searchTerm, + onChange, + dataCy +}) => ( + <Paper + sx={{ Review Comment: I've removed the `sx={{...}}` props in `SidebarSearchInput.tsx` and migrated the width, color, and padding styles to `.sidebar-searchbar` (and its nested elements) in `sidebar.scss`. ########## dashboard/src/redux/slice/sessionSlice.ts: ########## @@ -77,6 +96,30 @@ const sessionSlice = createSlice({ data: null, error: (action.payload as string) || action.error?.message || 'An error occurred' }; + }), + builder.addCase(fetchVersionData.pending, (state) => { Review Comment: I've updated `sessionSlice.ts` so that `data` is no longer cleared on pending, keeping the previous data available while it fetches (stale-while-revalidate). I've also added test cases in `sessionSlice.test.ts` to verify this behavior. ########## dashboard/src/views/SideBar/__tests__/SideBarBody.test.tsx: ########## @@ -15,6 +15,7 @@ * limitations under the License. */ +import '@testing-library/jest-dom'; Review Comment: I've updated `SideBarBody.tsx` to unconditionally render the tree components and used `display: open ? 'block' : 'none'` to keep them mounted when toggling the sidebar. I also updated the test suite in `SideBarBody.test.tsx` to explicitly verify that the components stay mounted in the document, which validates this core performance fix. ########## dashboard/src/components/__tests__/TreeSkeletonLoader.test.tsx: ########## @@ -0,0 +1,47 @@ +/* + * 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 { render } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import TreeSkeletonLoader from '../TreeSkeletonLoader'; + +describe('TreeSkeletonLoader', () => { + it('renders default number of skeletons when count is not provided', () => { + const { container } = render(<TreeSkeletonLoader />); + + // By default count is 7 + const skeletons = container.querySelectorAll('.MuiSkeleton-root'); + // Each row has 1 arrow, 1 text = 2 skeletons per row + // 7 rows * 2 = 14 skeletons + expect(skeletons.length).toBe(14); + }); + + it('renders specific number of skeletons based on count prop', () => { + const { container } = render(<TreeSkeletonLoader count={2} />); + + const skeletons = container.querySelectorAll('.MuiSkeleton-root'); + // 2 rows * 2 = 4 skeletons + expect(skeletons.length).toBe(4); + }); + + it('renders correctly with 0 count', () => { + const { container } = render(<TreeSkeletonLoader count={0} />); + + const skeletons = container.querySelectorAll('.MuiSkeleton-root'); + expect(skeletons.length).toBe(0); + }); Review Comment: I've updated `TreeSkeletonLoader.tsx` to handle negative counts safely (using `Math.max(0, count)`) and added corresponding edge-case tests in `TreeSkeletonLoader.test.tsx` for negative values and explicitly `undefined` counts. -- 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]
