base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[ScrollArea] Create new `ScrollArea` component

Open atomiks opened this issue 1 year ago • 5 comments

Closes #649

Preview: https://deploy-preview-665--base-ui.netlify.app/components/react-scroll-area/

atomiks avatar Sep 30 '24 21:09 atomiks

Netlify deploy preview

https://deploy-preview-665--base-ui.netlify.app/

Generated by :no_entry_sign: dangerJS against a9823fead215604ddf7089b4fe67268202cc1d7f

mui-bot avatar Sep 30 '24 21:09 mui-bot

It's about https://github.com/mui/base-ui/issues/1282, right? I have added the reference so developers can find a clear cross-link:

SCR-20241001-bikq

We could envision this component under the Base UI X brand umbrella in the future because most applications don't have a custom scrollbar. It seems more niche relative to nailing a menu, or combo box use case. Now, it's doesn't match with the criteria for Base UI X we defined (>=4 pages, >=1-2 person full time), still, I find it interesting that it shows the potential opportunity cost of having it in the core, or even open-source.

oliviertassinari avatar Sep 30 '24 22:09 oliviertassinari

@oliviertassinari

It's about https://github.com/mui/base-ui/issues/1282, right?

No we didn't see this issue prior to planning this work. It's just a basic building block of high-quality web dev, so I had planned to add it from the beginning.

most applications don't have a custom scrollbar

Very many web apps (Slack, Linear, Radix docs etc. etc.) have custom scrollbars. It's not possible to build a high-quality web UI without them.

We could envision this component under the Base UI X brand umbrella

Do you think this belongs under the premium components? We need it for our new docs (for the sidebar and code blocks). It's not a huge investment, and Radix provides one for free.

colmtuite avatar Sep 30 '24 22:09 colmtuite

No we didn't see this issue prior to planning this work. It's just a basic building block of high-quality web dev, so I had planned to add it from the beginning.

@colmtuite In any way, I mostly wanted to create a backlink between these two so it's easier to discover the link.

Very many web apps (Slack, Linear, Radix docs etc. etc.) have custom scrollbars. It's not possible to build a high-quality web UI without them.

For peak execution, I very much agree. For the average UIs, I don't think it's a big need, it's common to not see it used in average UIs, e.g. I personally never implemented a custom scrollbar 😄.

I think to be careful about the opportunity cost: downloads https://npm-stat.com/charts.html?package=overlayscrollbars&package=react-dropzone&from=2018-10-05&to=2024-10-05 vs. effort to do it right: https://github.com/KingSora/OverlayScrollbars/issues?q=is%3Aissue+is%3Aclosed.

The platform exposes some primitives to customize this, e.g. https://codepen.io/oliviertassinari/pen/PoMzdJa

https://github.com/user-attachments/assets/360c88c7-39a4-458e-bfc6-4f814251b37a

but until they fix https://github.com/w3c/csswg-drafts/issues/9826, https://github.com/w3c/csswg-drafts/issues/10591, it doesn't feel like a viable option, far from it.

Do you think this belongs under the premium components? We need it for our new docs (for the sidebar and code blocks). It's not a huge investment, and Radix provides one for free.

Premium plan: no. Pro plan: maybe for the styled layer it would make sense? It might be pushing it too far to have this as a pro feature in Base UI.

I get the feeling that 3 years from now, this component is no longer needed, built-in into the platform.

oliviertassinari avatar Oct 06 '24 20:10 oliviertassinari

The CSB version of the intro demo (https://codesandbox.io/p/sandbox/pydrhc?file=%2Fsrc%2Findex.tsx) looks slightly broken: image

Likely a box-sizing issue?


As for the implementation, I couldn't spot any issues. Good work!

michaldudak avatar Oct 11 '24 09:10 michaldudak

Took it for a spin. Many notes, but overall it's great. I love how much control I have over when exactly the scrollbar would be shown or hidden.

  • Are we sure we don't want to remove the built-in scrollbar-width and ::-webkit-scrollbar styles by default? Having to add them manually is going to be a hurdle when Radix users migrate, especially if they use Tailwind, which doesn't provide classes for either property.
  • type="inlay" is an unusual value. I appreciate the symmetry with "overlay" but I don't think I had ever typed the word "inlay" in the code editor, or referred to these scrollbars in a conversation like that. "inset" or "classic" is what I'd expect to call that scrollbar type instead.
    • Since we provide little built-in styles here already, I almost wonder if gutter and type should be implemented by the user? I assume the difference is mainly in having position: absolute and setting the padding manually. Where do we draw the line between functional styles like this and styles like removing default scrollbar-width that you seem to have to do in all cases?
  • Arrow keys don't scroll the scroll area after clicking the scrollbar (works with native scrollbars)
  • Scrollbar visibility and size doesn't update when Root height changes (e.g. due to an imperative style change. It does update when children change).
  • overscroll-behavior: none on Root doesn't affect the Scrollbar part. In other words, scrolling with pointer over the Scrollbar part retains the default overscroll behaviour. This is problematic in designs where overscroll-behavior is important to remove.
  • It was a bit unintuitive to figure out that to always display scrollbars, you'd need to use hidden={false} (I assume this is the way?). I expected keepMounted or something like that coming from Radix. Not a big deal, just seems to be slightly inconsistent with how force rendering other components works.
  • Should it be data-hovered to converge with React Aria's data attribute and our own data-pressed?

vladmoroz avatar Oct 15 '24 11:10 vladmoroz

Are we sure we don't want to remove the built-in scrollbar-width and ::-webkit-scrollbar styles by default? Having to add them manually is going to be a hurdle when Radix users migrate, especially if they use Tailwind, which doesn't provide classes for either property.

I think we can remove them by default, but Safari requires a <style> tag, right?

type="inlay" is an unusual value. I appreciate the symmetry with "overlay" but I don't think I had ever typed the word "inlay" in the code editor, or referred to these scrollbars in a conversation like that. "inset" or "classic" is what I'd expect to call that scrollbar type instead.

I got this naming from VS Code: it has a feature called "inlay hints" which are inline with the flow of the code

Since we provide little built-in styles here already, I almost wonder if gutter and type should be implemented by the user? I assume the difference is mainly in having position: absolute and setting the padding manually. Where do we draw the line between functional styles like this and styles like removing default scrollbar-width that you seem to have to do in all cases?

I think we can remove the scrollbar styles by default now, but removing the scrollbar is a lot easier than defining the padding (and the element node is not actually exposed by default).

Scrollbar visibility and size doesn't update when Root height changes (e.g. due to an imperative style change. It does update when children change).

There's a ResizeObserver but it might need to be lifted up to the root if it's defined on the viewport. Will check this.

overscroll-behavior: none on Root doesn't affect the Scrollbar part. In other words, scrolling with pointer over the Scrollbar part retains the default overscroll behaviour. This is problematic in designs where overscroll-behavior is important to remove.

I don't think this is possible to fix since it uses a wheel event, since the pointer events block scrolling otherwise. Radix also has this issue

It was a bit unintuitive to figure out that to always display scrollbars, you'd need to use hidden={false} (I assume this is the way?). I expected keepMounted or something like that coming from Radix. Not a big deal, just seems to be slightly inconsistent with how force rendering other components works.

They're visible by default, there's no hidden or anything. You can hide it by default then use the hovering/scrolling style hooks to show it.

Should it be data-hovered to converge with React Aria's data attribute and our own data-pressed?

hovering matches scrolling verb, but hover alone would match the CSS state. hovered sounds like you've potentially left the element already and doesn't seem accurate.

atomiks avatar Oct 15 '24 11:10 atomiks

I don't think this is possible to fix since it uses a wheel event, since the pointer events block scrolling otherwise. Radix also has this issue

Is this approach too heavy-handed? I asked specifically because I had run into the overscroll-behavior issue before with Radix and it's really unpleasant with no workaround.

I think we can remove them by default, but Safari requires a <style> tag, right?

Yeah Safari is the main problem. We don't have a precedent for rendering a <style> tag yet, do we?

They're visible by default, there's no hidden or anything. You can hide it by default then use the hovering/scrolling style hooks to show it.

I'm seeing a hidden attribute when there is no overflow:

image

I'd expect to either not have this element rendered at all and force mount it, or have to handle the visibility myself.

hovering matches scrolling verb, but hover alone would match the CSS state. hovered sounds like you've potentially left the element already and doesn't seem accurate.

I think it's best when data attributes describe the state of the component itself rather than what the user is doing; in this sense data-hovered reads more like "the element is hovered" to me, same as data-selected, data-active, etc. Can go either way on data-scrolling since it's a transitory state though. Maybe best if we leave these discussions for the Monday calls

vladmoroz avatar Oct 15 '24 12:10 vladmoroz

I'm seeing a hidden attribute when there is no overflow:

There are two separate "hidden" cases:

  1. There is no overflow, so no scrollbar should ever be shown - that's where hidden comes in
  2. There is overflow, but you need to decide when to show it: hovering, scrolling, or both?

I considered the option 1 to mean you never want to show it. The gutter prop keeps layout stable, so hidden is applied for that case (or we could just unmount the entire component).

Due to transitions/animations, it makes sense to keep it mounted at all times as they're easier to create this way. Unlike floating components that can contain expensive content and create lots of elements in the DOM, scrollbars are cheap. For consistency with other components, if we adopt the keepMounted API, it could be set to true by default.

Is https://github.com/radix-ui/primitives/pull/2200 too heavy-handed? I asked specifically because I had run into the overscroll-behavior issue before with Radix and it's really unpleasant with no workaround.

While this works for none/contain, it still doesn't support the browser's default auto value which supports native scroll-chaining. We could do some kind of detection here but it wouldn't be as good as the native implementation.

It looks like this also unfortunately breaks being able to create a "hit area" around the thumb as Radix does, in order to be able to click it more easily.

Edit: I think preventing scroll of the parent at all times regardless of overscroll-behavior might be the best solution. This ensures that the hit area can still work, and to me feels like a better trade-off for the default auto overscroll on the first momentum scroll/wheel try, even though it still blocks subsequent attempts to scroll.

Yeah Safari is the main problem. We don't have a precedent for rendering a

I don't think we've used it yet, but I suppose Radix's approach will work.

atomiks avatar Oct 16 '24 02:10 atomiks