Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3796114047
##########
dashboard/src/views/SideBar/SideBarTree/__tests__/SideBarTree.test.tsx:
##########
@@ -2181,4 +2188,76 @@ describe('SideBarTree', () => {
expect(mockSetIsEmptyServicetype).not.toHaveBeenCalled()
})
})
+
+ describe('Reviewer Requested Tests', () => {
+ it('should render skeleton loader with 2 rows when isPopover is
true', async () => {
+ renderComponent({ loader: true, isPopover: true })
+
+ await waitFor(() => {
+ const loader =
screen.getByTestId('tree-skeleton-loader')
+ expect(loader).toBeInTheDocument()
+ expect(loader).toHaveAttribute('data-count',
'2')
+ })
+ })
+
+ it('should persist selected state from URL params for custom
filters when reopened in popover', async () => {
Review Comment:
Resolved. Good catch! The test assertion was improperly relying on
expandedItems (which auto-expands on mount) and passing the wrong type= URL
parameter. I've updated the test to use the correct customFilter= parameter and
refactored the assertion to strictly verify the presence of the Mui-selected
CSS class on the tree node.
##########
dashboard/src/views/SideBar/SideBarTree/__tests__/SideBarTree.test.tsx:
##########
@@ -2181,4 +2188,76 @@ describe('SideBarTree', () => {
expect(mockSetIsEmptyServicetype).not.toHaveBeenCalled()
})
})
+
+ describe('Reviewer Requested Tests', () => {
+ it('should render skeleton loader with 2 rows when isPopover is
true', async () => {
+ renderComponent({ loader: true, isPopover: true })
+
+ await waitFor(() => {
+ const loader =
screen.getByTestId('tree-skeleton-loader')
+ expect(loader).toBeInTheDocument()
+ expect(loader).toHaveAttribute('data-count',
'2')
+ })
+ })
+
+ it('should persist selected state from URL params for custom
filters when reopened in popover', async () => {
+ const treeData = [
+ { id: 'customFilter1', label: 'Custom Filter
1', children: [] }
+ ]
+ renderComponent({
+ treeData,
+ treeName: 'CustomFilters',
+ isPopover: true
+ }, {},
['/search/searchResult?searchType=BASIC&isCF=true&type=customFilter1'])
Review Comment:
Resolved. I've updated the test to use the correct customFilter= parameter
and refactored the assertion to strictly verify the presence of the
Mui-selected CSS class on the tree node instead of checking the auto-expanded
items.
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -246,38 +360,117 @@ const SideBarBody = (props: {
backgroundColor: "#034858",
}}
>
- {/* Collapsed sidebar logo */}
+ {/* Collapsed sidebar logo and module icons */}
{!open && (
- <div
- style={{
- width: "100%",
- textAlign: "center",
- paddingLeft: "12px",
- display: "flex",
- alignItems: "center",
- justifyContent: "center",
- minHeight: "64px",
- cursor: "pointer",
- boxSizing: "border-box",
- }}
- role="button"
- tabIndex={0}
- aria-label="Atlas home — refresh dashboard"
- onClick={handleAtlasLogoClick}
- onKeyDown={handleAtlasLogoKeyDown}
- data-cy="apache-atlas-logo-collapsed"
+ <Stack
+ alignItems="center"
+ sx={{ width: "100%", flex: 1, minHeight: 0, overflowY: "auto",
overflowX: "hidden", boxSizing: "border-box", pb: "60px" }}
>
- <img
- src={apacheAtlasLogo}
- alt="Apache Atlas logo"
- style={{
- width: "29px",
- height: "auto",
- maxWidth: "100%",
- display: "block",
+ <div
+ className="collapsed-logo-container"
+ role="button"
+ tabIndex={0}
+ aria-label="Atlas home — refresh dashboard"
Review Comment:
Resolved. I've added aria-haspopup="dialog" to the module IconButtons for
better screen reader support. Note that aria-expanded={activePopover === m.id}
was actually already implemented and correctly toggling on the buttons!
--
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]
