From bec7911d59bc465e0cef563c947491d65e0e1385 Mon Sep 17 00:00:00 2001 From: nitin <142569587+ehconitin@users.noreply.github.com> Date: Wed, 8 Jan 2025 19:33:47 +0530 Subject: [PATCH] Navigation drawer scroll padding fix (#9141) closes https://github.com/twentyhq/twenty/issues/9026 fixes #9312 https://github.com/user-attachments/assets/3d7df3ec-8a5e-4308-8993-82c715edc683 --------- Co-authored-by: Lucas Bordeau --- .../components/MainNavigationDrawerItems.tsx | 15 ++-- .../components/RecordTableAggregateFooter.tsx | 16 +++- .../components/NavigationDrawer.tsx | 5 +- .../components/NavigationDrawerItem.tsx | 2 +- .../components/NavigationDrawerSection.tsx | 24 ++++- .../scroll/components/ScrollWrapper.tsx | 87 ++++++++++--------- 6 files changed, 100 insertions(+), 49 deletions(-) diff --git a/packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx b/packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx index da9d15131..e42e647a0 100644 --- a/packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx +++ b/packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx @@ -19,6 +19,9 @@ import styled from '@emotion/styled'; const StyledMainSection = styled(NavigationDrawerSection)` min-height: fit-content; `; +const StyledInnerContainer = styled.div` + height: 100%; +`; export const MainNavigationDrawerItems = () => { const isMobile = useIsMobile(); @@ -60,12 +63,14 @@ export const MainNavigationDrawerItems = () => { contextProviderName="navigationDrawer" componentInstanceId={`scroll-wrapper-navigation-drawer`} defaultEnableXScroll={false} - scrollHide={true} + scrollbarVariant="no-padding" > - - - - + + + + + + ); diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter.tsx index d4bf5358f..709643e01 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter.tsx @@ -62,7 +62,21 @@ const StyledTableFoot = styled.thead<{ endOfTableSticky?: boolean }>` tr { position: sticky; z-index: 5; - ${({ endOfTableSticky }) => endOfTableSticky && `bottom: 0;`} + background: ${({ theme }) => theme.background.primary}; + ${({ endOfTableSticky }) => + endOfTableSticky && + ` + bottom: 10px; + &::after { + content: ''; + position: absolute; + bottom: -10px; + left: 0; + right: 0; + height: 10px; + background: inherit; + } + `} } `; diff --git a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawer.tsx b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawer.tsx index 75c645db2..6572b66cf 100644 --- a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawer.tsx +++ b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawer.tsx @@ -10,6 +10,7 @@ import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; import { NAV_DRAWER_WIDTHS } from '@/ui/navigation/navigation-drawer/constants/NavDrawerWidths'; import { useIsSettingsDrawer } from '@/navigation/hooks/useIsSettingsDrawer'; +import { NavigationDrawerSection } from '@/ui/navigation/navigation-drawer/components/NavigationDrawerSection'; import { isNavigationDrawerExpandedState } from '../../states/isNavigationDrawerExpanded'; import { NavigationDrawerBackButton } from './NavigationDrawerBackButton'; import { NavigationDrawerHeader } from './NavigationDrawerHeader'; @@ -43,7 +44,7 @@ const StyledContainer = styled.div<{ ? theme.spacing(3, 8) : theme.spacing(3, 8, 4, 0) : theme.spacing(3, 2, 4)}; - + padding-right: 0px; @media (max-width: ${MOBILE_VIEWPORT}px) { width: 100%; padding-left: 20px; @@ -123,7 +124,7 @@ export const NavigationDrawer = ({ {children} - {footer} + {footer} ); diff --git a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx index cfab135f8..bd9c6b5e5 100644 --- a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx +++ b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx @@ -92,7 +92,7 @@ const StyledItem = styled('button', { width: ${(props) => !props.isNavigationDrawerExpanded ? `calc(${NAV_DRAWER_WIDTHS.menu.desktop.collapsed}px - ${props.theme.spacing(5.5)})` - : `calc(100% - ${props.theme.spacing(2)})`}; + : `calc(100% - ${props.theme.spacing(1.5)})`}; ${({ isDragging }) => isDragging && diff --git a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerSection.tsx b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerSection.tsx index 9de5b7004..4e3711716 100644 --- a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerSection.tsx +++ b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerSection.tsx @@ -1,4 +1,5 @@ import styled from '@emotion/styled'; +import { useIsMobile } from 'twenty-ui'; const StyledSection = styled.div` display: flex; @@ -9,4 +10,25 @@ const StyledSection = styled.div` flex-shrink: 1; `; -export { StyledSection as NavigationDrawerSection }; +const StyledSectionInnerContainerMinusScrollPadding = styled.div<{ + isMobile: boolean; +}>` + width: calc( + 100% - ${({ isMobile, theme }) => (isMobile ? 0 : theme.spacing(2))} + ); +`; + +export const NavigationDrawerSection = ({ + children, +}: { + children: React.ReactNode; +}) => { + const isMobile = useIsMobile(); + return ( + + + {children} + + + ); +}; diff --git a/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx b/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx index 949a5d010..bdc131a28 100644 --- a/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx +++ b/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx @@ -13,13 +13,14 @@ import { scrollWrapperInstanceComponentState } from '@/ui/utilities/scroll/state import { scrollWrapperScrollLeftComponentState } from '@/ui/utilities/scroll/states/scrollWrapperScrollLeftComponentState'; import { scrollWrapperScrollTopComponentState } from '@/ui/utilities/scroll/states/scrollWrapperScrollTopComponentState'; import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; +import { css } from '@emotion/react'; import 'overlayscrollbars/overlayscrollbars.css'; type HeightMode = 'full' | 'fit-content'; const StyledScrollWrapper = styled.div<{ - scrollHide?: boolean; heightMode: HeightMode; + scrollbarVariant: 'with-padding' | 'no-padding'; }>` display: flex; height: ${({ heightMode }) => { @@ -33,9 +34,38 @@ const StyledScrollWrapper = styled.div<{ width: 100%; .os-scrollbar-handle { - background-color: ${({ theme, scrollHide }) => - scrollHide ? 'transparent' : theme.border.color.medium}; + background-color: ${({ theme }) => theme.border.color.strong}; } + + // Keep horizontal scrollbar always visible + .os-scrollbar-horizontal { + &.os-scrollbar-auto-hide { + opacity: 1; + visibility: visible; + } + .os-scrollbar-track { + visibility: visible !important; + } + } + + .os-scrollbar { + transition: + opacity 300ms, + visibility 300ms, + top 300ms, + right 300ms, + bottom 300ms, + left 300ms; + } + + ${({ scrollbarVariant }) => + scrollbarVariant === 'no-padding' && + css` + .os-scrollbar { + --os-size: 6px; + padding: 0px; + } + `} `; const StyledInnerContainer = styled.div` @@ -49,8 +79,8 @@ export type ScrollWrapperProps = { defaultEnableXScroll?: boolean; defaultEnableYScroll?: boolean; contextProviderName: ContextProviderName; - scrollHide?: boolean; componentInstanceId: string; + scrollbarVariant?: 'with-padding' | 'no-padding'; }; export const ScrollWrapper = ({ @@ -61,7 +91,7 @@ export const ScrollWrapper = ({ defaultEnableXScroll = true, defaultEnableYScroll = true, contextProviderName, - scrollHide = false, + scrollbarVariant = 'with-padding', }: ScrollWrapperProps) => { const scrollableRef = useRef(null); const Context = getContextByProviderName(contextProviderName); @@ -94,39 +124,23 @@ export const ScrollWrapper = ({ autoHideDelay: 500, }, overflow: { - x: defaultEnableXScroll ? 'scroll' : 'hidden', - y: defaultEnableYScroll ? 'scroll' : 'hidden', + x: defaultEnableXScroll ? undefined : 'hidden', + y: defaultEnableYScroll ? undefined : 'hidden', }, }, events: { scroll: (osInstance) => { - const { - scrollOffsetElement: target, - scrollbarHorizontal, - scrollbarVertical, - } = osInstance.elements(); + const { scrollOffsetElement: target, scrollbarVertical } = + osInstance.elements(); + // Hide vertical scrollbar by default + if (scrollbarVertical !== null) { + scrollbarVertical.track.style.visibility = 'hidden'; + } - // Hide scrollbars by default - [scrollbarHorizontal, scrollbarVertical].forEach((scrollbar) => { - if (scrollbar !== null) { - scrollbar.track.style.visibility = 'hidden'; - } - }); - - // Show appropriate scrollbar based on scroll direction - const isHorizontalScroll = - target.scrollLeft !== Number(target.dataset.lastScrollLeft || '0'); + // Show vertical scrollbar based on scroll direction const isVerticalScroll = target.scrollTop !== Number(target.dataset.lastScrollTop || '0'); - // Show scrollbar based on scroll direction only with explicit conditions - if ( - isHorizontalScroll === true && - scrollbarHorizontal !== null && - target.scrollWidth > target.clientWidth - ) { - scrollbarHorizontal.track.style.visibility = 'visible'; - } if ( isVerticalScroll === true && scrollbarVertical !== null && @@ -134,9 +148,7 @@ export const ScrollWrapper = ({ ) { scrollbarVertical.track.style.visibility = 'visible'; } - - // Update scroll positions - target.dataset.lastScrollLeft = target.scrollLeft.toString(); + // Update vertical scroll positions target.dataset.lastScrollTop = target.scrollTop.toString(); handleScroll(osInstance); @@ -146,19 +158,16 @@ export const ScrollWrapper = ({ useEffect(() => { const currentRef = scrollableRef.current; - if (currentRef !== null) { initialize(currentRef); } - return () => { - // Reset all component-specific Recoil state + // Reset vertical scroll component-specific Recoil state setScrollTop(0); - setScrollLeft(0); setOverlayScrollbars(null); instance()?.destroy(); }; - }, [initialize, instance, setScrollTop, setScrollLeft, setOverlayScrollbars]); + }, [initialize, instance, setScrollTop, setOverlayScrollbars]); useEffect(() => { setOverlayScrollbars(instance()); @@ -177,8 +186,8 @@ export const ScrollWrapper = ({ {children}