Skip to content

Commit

Permalink
Navigation drawer scroll padding fix (#9141)
Browse files Browse the repository at this point in the history
  • Loading branch information
ehconitin and lucasbordeau authored Jan 8, 2025
1 parent 428572a commit bec7911
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -60,12 +63,14 @@ export const MainNavigationDrawerItems = () => {
contextProviderName="navigationDrawer"
componentInstanceId={`scroll-wrapper-navigation-drawer`}
defaultEnableXScroll={false}
scrollHide={true}
scrollbarVariant="no-padding"
>
<NavigationDrawerOpenedSection />
<CurrentWorkspaceMemberFavoritesFolders />
<WorkspaceFavorites />
<RemoteNavigationDrawerSection />
<StyledInnerContainer>
<NavigationDrawerOpenedSection />
<CurrentWorkspaceMemberFavoritesFolders />
<WorkspaceFavorites />
<RemoteNavigationDrawerSection />
</StyledInnerContainer>
</ScrollWrapper>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
`}
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -123,7 +124,7 @@ export const NavigationDrawer = ({
<StyledItemsContainer isSettings={isSettingsDrawer}>
{children}
</StyledItemsContainer>
{footer}
<NavigationDrawerSection>{footer}</NavigationDrawerSection>
</StyledContainer>
</StyledAnimatedContainer>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import styled from '@emotion/styled';
import { useIsMobile } from 'twenty-ui';

const StyledSection = styled.div`
display: flex;
Expand All @@ -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 (
<StyledSection>
<StyledSectionInnerContainerMinusScrollPadding isMobile={isMobile}>
{children}
</StyledSectionInnerContainerMinusScrollPadding>
</StyledSection>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand All @@ -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`
Expand All @@ -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 = ({
Expand All @@ -61,7 +91,7 @@ export const ScrollWrapper = ({
defaultEnableXScroll = true,
defaultEnableYScroll = true,
contextProviderName,
scrollHide = false,
scrollbarVariant = 'with-padding',
}: ScrollWrapperProps) => {
const scrollableRef = useRef<HTMLDivElement>(null);
const Context = getContextByProviderName(contextProviderName);
Expand Down Expand Up @@ -94,49 +124,31 @@ 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();

// 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');
const { scrollOffsetElement: target, scrollbarVertical } =
osInstance.elements();
// Hide vertical scrollbar by default
if (scrollbarVertical !== null) {
scrollbarVertical.track.style.visibility = 'hidden';
}

// 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 &&
target.scrollHeight > target.clientHeight
) {
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);
Expand All @@ -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());
Expand All @@ -177,8 +186,8 @@ export const ScrollWrapper = ({
<StyledScrollWrapper
ref={scrollableRef}
className={className}
scrollHide={scrollHide}
heightMode={heightMode}
scrollbarVariant={scrollbarVariant}
>
<StyledInnerContainer>{children}</StyledInnerContainer>
</StyledScrollWrapper>
Expand Down

0 comments on commit bec7911

Please sign in to comment.