Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3543914142
##########
dashboard/src/views/SideBar/__tests__/SideBarBody.test.tsx:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
Review Comment:
Resolved. Added comprehensive test coverage to address all requested
behaviors:
**Collapsed module icon opens correct popover:** Added test verifying
glossary-tree renders when glossary icon is clicked.
**Active state markers:** Added test verifying active state styles are
applied based on route.
Popover closes on outside click/navigation: Added test verifying MUI Popover
unmounts when Escape/outside click is triggered.
**Search term shared:** Added test verifying search term state propagates
from the main sidebar to the popover's searchTerm prop.
**TreeSkeletonLoader:** Created TreeSkeletonLoader.test.tsx and verified it
renders the correct dynamic skeleton row count based on the count prop.
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -246,38 +380,227 @@ 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: 2, '&::-webkit-scrollbar': {
display: 'none' }, msOverflowStyle: 'none', scrollbarWidth: 'none' }}
>
- <img
- src={apacheAtlasLogo}
- alt="Apache Atlas logo"
+ <div
style={{
- width: "29px",
- height: "auto",
- maxWidth: "100%",
- display: "block",
+ width: "100%",
+ textAlign: "center",
+ display: "flex",
+ alignItems: "center",
+ justifyContent: "center",
+ minHeight: "64px",
+ cursor: "pointer",
+ boxSizing: "border-box",
+ marginBottom: "1rem",
}}
- />
- </div>
+ role="button"
+ tabIndex={0}
+ aria-label="Atlas home — refresh dashboard"
+ onClick={handleAtlasLogoClick}
+ onKeyDown={handleAtlasLogoKeyDown}
+ data-cy="apache-atlas-logo-collapsed"
+ >
+ <img
+ src={apacheAtlasLogo}
+ alt="Apache Atlas logo"
+ style={{
+ width: "29px",
+ height: "auto",
+ maxWidth: "100%",
+ display: "block",
+ }}
+ />
+ </div>
+
+ {/* Module Icons for Mini Drawer */}
+ <Stack alignItems="stretch" gap="1rem" sx={{ width: "100%" }}>
+ {/* Search */}
+ <Box sx={{ display: "flex", justifyContent: "center",
borderLeft: "4px solid transparent", borderRight: "4px solid transparent",
background: "transparent" }}>
+ <Tooltip title="Search" placement="right">
+ <IconButton onClick={() => setOpen(true)} sx={{ '&:hover':
{ background: 'rgba(255, 255, 255, 0.1)' } }}>
+ <img src="/img/sidebar-icons/icon-search.svg" style={{
width: "20px", height: "20px", opacity: 1 }} alt="search" />
+ </IconButton>
+ </Tooltip>
+ </Box>
+
+ {/* Entities */}
+ <Box sx={{ display: "flex", justifyContent: "center",
borderLeft: isEntitiesActive ? "4px solid #2ccebb" : "4px solid transparent",
borderRight: "4px solid transparent", background: isEntitiesActive ? "rgba(255,
255, 255, 0.08)" : "transparent" }}>
+ <Tooltip title="Entities" placement="right">
+ <IconButton onClick={(e) => handlePopoverOpen(e,
"entities")} sx={{ '&:hover': { background: 'rgba(255, 255, 255, 0.1)' } }}>
+ <img src="/img/sidebar-icons/icon-entities.svg" style={{
width: "20px", height: "20px", opacity: 1 }} alt="entities" />
+ </IconButton>
+ </Tooltip>
+ </Box>
+ <Popover
+ marginThreshold={64}
+ open={activePopover === "entities"}
+ anchorEl={popoverAnchor}
+ onClose={handlePopoverClose}
+ anchorOrigin={{ vertical: 'top', horizontal: 'right' }}
+ transformOrigin={{ vertical: 'top', horizontal: 'left' }}
+ PaperProps={{ sx: { ml: 1, width: 320, maxHeight:
'calc(100vh - 250px) !important', display: 'flex', flexDirection: 'column',
backgroundColor: '#034858', borderRadius: 1, boxShadow: 6, pb: 2, overflow:
'visible', '&::before': { content: '""', display: 'block', position:
'absolute', top: 14, left: -8, width: 0, height: 0, borderTop: '8px solid
transparent', borderBottom: '8px solid transparent', borderRight: '8px solid
#034858' } } }}
+ >
+ {renderPopoverSearch()}
+ <div style={{ flex: 1, overflow: 'auto' }}>
+ <Suspense fallback={<TreeSkeletonLoader count={2} />}>
+ <div className="sidebar-treeview-container" style={{
padding: '8px' }}>
+ <EntitiesTree sideBarOpen={true}
searchTerm={searchTerm} isPopover={true} />
Review Comment:
Resolved. Implemented exactly as suggested! Instead of letting MUI keep the
children mounted in the background, the lazy-loaded tree components are now
conditionally rendered inside the Popover using activePopover === m.id. This
ensures we completely avoid any unnecessary background lazy-loading or Redux
effect subscriptions until the popover is actually opened.
--
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]
