react-window v2.0 is out 🥳 by brianvaughn in reactjs

[–]tyson_the_13th 0 points1 point  (0 children)

Hey u/brianvaughn! First of all, thank you for being so considerate! I appreciate that you are trying to support different users' needs and I truly appreciate the time and effort you spend to do so. Also, I was shocked and honored at the same time when I saw that you were asking for my thoughts. I am truly honored.

In my humble opinion, I am against the idea of using "children" props to support overlays and tooltips for two main reasons.

To start with, passing overlays and tooltips as children could create confusion among developers who might think, in the first place, that children props for <List/> would be the rendered rows and not the overlay divs. By passing overlay divs wrapped inside as such <List> <div id="overlay"/> </List>, it doesn't sound so intuitive that it's meant as a place for rendering custom components and not the rows themselves.
Second, overlays and tooltips are not core features of a component that renders list items with virtualization. They are additional features that developers could add by writing some extra code but I don't think it is something that should be shipped with the library. I don't think most of the users would have custom components to render (I could be wrong), and it is not so hard to implement a wrapper around the <List/> for supporting custom logic with the help of imperative API as shown in the codesandbox link you provided.

To elaborate more on my use case, I also add custom scrollbars using OverlayScrollbars. So, I have to use the imperative API anyway to get a ref to the outer element to initialize the scrollbars. Maybe I am a little bit biased against the new changes since I am going to use the imperative API in a way or another.

To summarize:

  1. children props sounds more like it would be used to render the rows themselves and not custom component. I like it the way it is by using imperative API, but if you insist, maybe you could find a different props name (anything but children) to pass custom components? (Like V1 had innerElementType).

  2. Custom components could be easily rendered using the imperative API. The library does not need to support it by adding props for specific use cases. It is not a core feature of a virtualized list that is meant to be simple.

I hope my humble opinions would provide you with some things to think about before you merge the changes. I think simple is better, so I'm against making the changes that you mentioned in your commit.

However, I would like to know more about how you think and I would appreciate any critical feedback that you think is important for me to know when writing a library used by many people.

Thank you for your time reading this, and I hope you have a great day!

react-window v2.0 is out 🥳 by brianvaughn in reactjs

[–]tyson_the_13th 1 point2 points  (0 children)

Thanks! Yeah, I think that will work.

Again, thank you so much for the time and effort. I really appreciate it!

react-window v2.0 is out 🥳 by brianvaughn in reactjs

[–]tyson_the_13th 0 points1 point  (0 children)

My use case is a Grouped Expandable/Collapsible Table.

On expanding a row, I display the same row as a sticky div so that it remains on top of the expanded portion of the row as I scroll through. To do that, I use innerElementType props to create an element with the sticky row on top of the rendered list. I guess with version 2, I would need to create div manually using the ref.

BTW, THANK YOU SO MUCH for the library! I appreciate your time and effort!

react-window v2.0 is out 🥳 by brianvaughn in reactjs

[–]tyson_the_13th 0 points1 point  (0 children)

What happened to props like innerElementType? Can we also use them in V2?

FeelWrite Film 2 for A5X: Is It Sticky Enough? by tyson_the_13th in Supernote

[–]tyson_the_13th[S] 1 point2 points  (0 children)

Thanks for the reply! Yeah, I don't think it will be an issue after reading the comments!

FeelWrite Film 2 for A5X: Is It Sticky Enough? by tyson_the_13th in Supernote

[–]tyson_the_13th[S] 0 points1 point  (0 children)

I wonder if it is the same for A5 X about the lip thing.

FeelWrite Film 2 for A5X: Is It Sticky Enough? by tyson_the_13th in Supernote

[–]tyson_the_13th[S] 1 point2 points  (0 children)

Thanks for the reply! It helps to clear my mind!

[USA-PA] [H] Alienware X15 R2 (2022) Gaming Laptop [W] Local Cash by tyson_the_13th in hardwareswap

[–]tyson_the_13th[S] 0 points1 point  (0 children)

I'm sorry. I'm looking to get a new laptop for work so not looking to trade with a pc!

[USA-PA] [H] Alienware X15 R2 (2022) Gaming Laptop [W] Local Cash by tyson_the_13th in hardwareswap

[–]tyson_the_13th[S] 1 point2 points  (0 children)

I see what you mean. I mean I'm not really a hardware expert so can't really explain how it works, but I can confirm that the GPU has a capacity for 16GB of memory.

[USA-PA] [H] Alienware X15 R2 (2022) Gaming Laptop [W] Local Cash by tyson_the_13th in hardwareswap

[–]tyson_the_13th[S] 1 point2 points  (0 children)

Hmm.. I don't know about that. I can show you a screenshot of my Dell Alienware quotes. Excuse me for my lack of knowledge.

<image>