Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3827431163
##########
dashboard/src/styles/sidebar.scss:
##########
@@ -49,10 +49,10 @@
flex-grow: 1;
// position: fixed;
Review Comment:
Fixed.
1. **Shared Variables:** Extracted `#034858`, `#2ccebb`, and `#4a90e2` to
`variables.scss`. To cleanly bridge TSX and SCSS, these are now injected as CSS
variables in `:root` (e.g. `var(--sidebar-bg)`) and used directly in the MUI
`sx` props and `sidebar.scss` without duplicating hardcoded hex colors.
2. **Dead Code:** Cleaned up and removed all the commented-out CSS blocks
(lines 23, 48-52, 71-75).
3. **SidebarSearchInput Padding Conflict:** Removed `paddingLeft`,
`display`, and `alignItems` from the inline MUI `sx` prop in
`SidebarSearchInput.tsx` since they are now properly handled natively inside
`.sidebar-searchbar` without collisions.
4. **Unused Class:** Completely removed the redundant
`.sidebar-tree-label-nowrap` class. I migrated its `white-space: nowrap`
directive directly into `.tree-item-label` which is correctly utilized by the
`SideBarTree.tsx`.
5. **Hardcoded Colors:** Replaced all the remaining hardcoded hexes
(`#f1f1f1`, `#ddd`, `#D3D3D3`, `#686868`) inside `sidebar.scss` with cleanly
defined `variables.scss` tokens.
##########
dashboard/src/views/SideBar/__tests__/SideBarBody.test.tsx:
##########
Review Comment:
Fixed. I've added the missing tests to ensure these states are properly
validated:
1. **Version Footer:** Added dedicated assertions for `loading spinner`
presence during `isVersionLoading` and correct rendering of the `V x.x` format.
The 'Version unavailable' error state was already covered but has been verified.
2. **Relationships Module (Negative Test):** Added an assertion to guarantee
both the mini-sidebar icon and the expanded `RelationshipsTree` are correctly
suppressed when `relationshipSearch` is falsy.
3. **Popover Triggers:** Added a test verifying that module icon triggers do
NOT render (and cannot accidentally trigger popovers) while the sidebar is
completely expanded.
4. **Popover Exclusivity:** Added a test that clicks the Glossary module
then the Entities module, verifying only the newly clicked popover remains open
while the previous one unmounts.
5. **relationshipSearch Mock:** The `globalSessionData` mock was updated
from `{}` to `true` in a previous commit, removing the false positive and
forcing us to write explicit `true`/`false` tests for it.
##########
dashboard/src/views/SideBar/SideBarTree/__tests__/SideBarTree.test.tsx:
##########
@@ -27,6 +27,7 @@
*/
Review Comment:
Fixed. Removed the `TreeLabelWithTooltip` block from lines 1091–1110 as it
only asserted rendering and didn't test the actual JS-based `scrollWidth` /
`clientWidth` behavior. The exact functionality is properly and strongly tested
in the later blocks.
--
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]
