feat(scroll btn): Add SelectScrollButton for scrolling dropdown, visual overflow indication #7579
Conversation
…al overflow indicator
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Could you add stories for this component ?
There was a problem hiding this comment.
(Keep in mind, that, ideally, the stories will show this component working within a Select component, and not on its own)
| @@ -0,0 +1,130 @@ | |||
| import { ChevronDownIcon, ChevronUpIcon } from '@heroicons/react/24/outline'; | |||
| import { useEffect, useRef, useState } from 'react'; | |||
| import { type FC, type RefObject } from 'react'; | |||
There was a problem hiding this comment.
| import { type FC, type RefObject } from 'react'; | |
| import type { FC, RefObject } from 'react'; |
| import { useEffect, useRef, useState } from 'react'; | ||
| import { type FC, type RefObject } from 'react'; | ||
|
|
||
| import styles from '@node-core/ui-components/Common/Select/index.module.css'; |
There was a problem hiding this comment.
| import styles from '@node-core/ui-components/Common/Select/index.module.css'; | |
| import styles from '@node-core/ui-components/Common/Select/SelectScrollButton/index.module.css'; |
Maybe use dedicate css-module to simplify code
| direction, | ||
| selectContentRef, | ||
| scrollAmount = 35, | ||
| scrollInterval = 50, |
There was a problem hiding this comment.
| scrollInterval = 50, | |
| scrollInterval = 500, |
for moment it's to quick
| .scrollBtn { | ||
| @apply sticky | ||
| z-10 | ||
| flex | ||
| w-full | ||
| cursor-pointer | ||
| justify-center | ||
| bg-white | ||
| p-1 | ||
| transition-colors | ||
| hover:bg-neutral-100 | ||
| dark:bg-neutral-950 | ||
| dark:hover:bg-neutral-900; | ||
| } | ||
|
|
||
| .scrollBtn[data-direction='down'] { | ||
| bottom: 0; | ||
| } | ||
|
|
||
| .scrollBtn[data-direction='up'] { | ||
| top: 0; | ||
| } | ||
|
|
||
| .scrollBtnIcon { | ||
| @apply size-5 | ||
| text-neutral-600 | ||
| dark:text-neutral-400; | ||
| } |
There was a problem hiding this comment.
| .scrollBtn { | |
| @apply sticky | |
| z-10 | |
| flex | |
| w-full | |
| cursor-pointer | |
| justify-center | |
| bg-white | |
| p-1 | |
| transition-colors | |
| hover:bg-neutral-100 | |
| dark:bg-neutral-950 | |
| dark:hover:bg-neutral-900; | |
| } | |
| .scrollBtn[data-direction='down'] { | |
| bottom: 0; | |
| } | |
| .scrollBtn[data-direction='up'] { | |
| top: 0; | |
| } | |
| .scrollBtnIcon { | |
| @apply size-5 | |
| text-neutral-600 | |
| dark:text-neutral-400; | |
| } | |
| .scrollBtn { | |
| @apply sticky | |
| z-10 | |
| flex | |
| w-full | |
| cursor-pointer | |
| justify-center | |
| bg-white | |
| p-1 | |
| transition-colors | |
| hover:bg-neutral-100 | |
| dark:bg-neutral-950 | |
| dark:hover:bg-neutral-900; | |
| &[data-direction='down'] { | |
| @apply bottom-0; | |
| } | |
| &[data-direction='up'] { | |
| @apply top-0 | |
| } | |
| .scrollBtnIcon { | |
| @apply size-5 | |
| text-neutral-600 | |
| dark:text-neutral-400; | |
| } | |
| } |
you can use nested css to make it easier to maintain
| if (direction === 'down') { | ||
| container.scrollBy({ top: scrollAmount, behavior: 'smooth' }); | ||
|
|
||
| if ( | ||
| container.scrollTop >= | ||
| container.scrollHeight - container.clientHeight | ||
| ) { | ||
| clearScrollInterval(); | ||
| setIsVisible(false); | ||
| } | ||
| } else { | ||
| container.scrollBy({ | ||
| top: -Math.abs(scrollAmount), | ||
| behavior: 'smooth', | ||
| }); | ||
|
|
||
| if (container.scrollTop <= 0) { | ||
| clearScrollInterval(); | ||
| setIsVisible(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is an extremely extremely minor nitpick, but this could probably be simplified into a helper function?
(But this is such a tiny nitpick that you really don't have to do anything)
| justify-center | ||
| bg-white | ||
| p-1 | ||
| transition-colors |
There was a problem hiding this comment.
| transition-colors | |
| motion-safe:transition-colors |
| scrollInterval?: number; | ||
| }; | ||
|
|
||
| const SelectScrollButton: FC<SelectScrollButtonProps> = ({ |
There was a problem hiding this comment.
Primitive radix select already has ScrollUpButton and ScrollDownButton Instead of adding these features, you can use what radix provides directly. Here is an example of primitive select anatomy; https://www.radix-ui.com/primitives/docs/components/select#anatomy
|
The up chevron doesn't ever appear to show, can you investigate? It looks like only the down arrow shows. |
Description
This PR introduces a new component SelectScrollButton imported into the Select component that tracks the content/scrollability of the Select component and conditionally renders a scrolldown button that scrolls the select element down when hovered over as well as serving as a visual indicator of overflow since certain browsers opt to hide scrollbars. I tried first to use the ScrollDownButton component as suggested by user 'shadowspawn' in the issue thread, but it seems that it's not designed to be used alongside the component ScrollArea(ScrollPrimitive) while inside the Select component and does not render anything in that scenario. The behavior of the new component is closely matched to that of Radix UI's Select.ScrollDownButton. Currently it only works for scrolling down and I wanted to make sure this approach is acceptable before moving forward and potentially giving it the ability to stick to the top of the container and scroll up as well as down. This is my first contribution to this repository, please let me know if I missed anything or if you think it's necessary for me to add tests for this component, etc.
Validation
I manually tested the dropdowns on the downloads page on my (windows 10) desktop on firefox, chrome and edge as well as my android phone and it appears to function as intended.
Related Issues
#7468
Check List
npm run formatto ensure the code follows the style guide.npm run testto check if all tests are passing.npx turbo buildto check if the website builds without errors.