Dropdowns should have a max-height and a scroll bar

FelixMalfait commented 2 months ago

I was shadowing a client that had created lots of field and the Filter dropdown was bigger than their screen's height. We might (or might not?) want to set a max-height. Also we want to make sure there's a scroll bar upon scroll (when I added a max-height directly in Chrome, the dropdown become scrollable but I couldn't see any scroll bar)

cc @Bonapara for recommandation

greptile-apps[bot] commented 2 months ago

To address the issue of dropdowns being larger than the screen height and ensuring a visible scrollbar, follow these steps:

Update DropdownMenuItemsContainer.tsx:

const StyledDropdownMenuItemsExternalContainer = styled.div<{
  hasMaxHeight?: boolean;
}>`
  --padding: ${({ theme }) => theme.spacing(1)};

  align-items: flex-start;
  display: flex;

  flex-direction: column;
  gap: 2px;
  height: 100%;
  max-height: ${({ hasMaxHeight }) => (hasMaxHeight ? '188px' : 'none')};
  overflow-y: auto;
  overflow-y: scroll; /* Ensure scrollbar is visible */

  padding: var(--padding);

  width: calc(100% - 2 * var(--padding));
`;

Update DropdownMenu.tsx:

const StyledDropdownMenu = styled.div<{
  disableBlur?: boolean;
  disableBorder?: boolean;
  width?: `${string}px` | `${number}%` | 'auto' | number;
}>`
  backdrop-filter: ${({ theme, disableBlur }) =>
    disableBlur ? 'none' : theme.blur.medium};

  color: ${({ theme }) => theme.font.color.secondary};

  background: ${({ theme, disableBlur }) =>
    disableBlur
      ? theme.background.primary
      : theme.background.transparent.primary};

  border: ${({ disableBorder, theme }) =>
    disableBorder ? 'none' : `1px solid ${theme.border.color.medium}`};
  border-radius: ${({ theme }) => theme.border.radius.md};

  box-shadow: ${({ theme }) => theme.boxShadow.strong};

  display: flex;

  flex-direction: column;
  z-index: 1;
  width: ${({ width = 160 }) =>
    typeof width === 'number' ? `${width}px` : width};
  max-height: 188px; /* Set max-height */
  overflow-y: auto; /* Ensure scrollbar is visible */
`;

These changes will ensure that the dropdowns have a max-height and a visible scrollbar when the content exceeds the specified height.

References

/packages/twenty-front/src/modules/ui/layout/dropdown/components/DropdownMenuItemsContainer.tsx /packages/twenty-front/src/modules/ui/layout/dropdown/components/DropdownMenu.tsx /packages/twenty-front/src/modules/ui/input/components/Select.tsx /packages/twenty-front/src/modules/ui/layout/dropdown/components/Dropdown.tsx

#### About Greptile

This response provides a starting point for your research, not a precise solution. Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant. [Ask Greptile](https://app.greptile.com/chat/github/twentyhq/twenty/main) · [Edit Issue Bot Settings](https://app.greptile.com/apps/github)

Bonapara commented 2 months ago

The max-height should be automatically determined based on the screen height. We should always maintain a minimum 20px bottom margin between the menu and the screen bottom:

cooldude6000 commented 2 months ago

Is this issue open ? Can i take it?

Bonapara commented 2 months ago

Sure! Thanks for contributing @gowreesh369

Faisal-imtiyaz123 commented 2 months ago

@cooldude6000 are you working here?

Bonapara commented 2 months ago

Just assigned you @Faisal-imtiyaz123, thanks for contributing!

Faisal-imtiyaz123 commented 2 months ago

@Bonapara Thanks for your quick response as always!

Faisal-imtiyaz123 commented 1 month ago

@Bonapara What should be scroll bar dimensions and color?