iTwin / iTwinUI-react

A react component library for iTwinUI.
https://github.com/iTwin/iTwinUI
Other
83 stars 23 forks source link

Table: add ScrollToRow / ScrollToIndex #558

Closed Nmzik closed 2 years ago

Nmzik commented 2 years ago

Add a method, which scrolls the item at the specified index into view. Optional parameters:

veekeys commented 2 years ago

Hey @Nmzik,

Thank you for the issue! Not sure we would prioritize this very high right now compared to other issues, but I put together this sandbox, which you could easily implement in your app.

https://codesandbox.io/s/itwinui-react-table-scrollintoview-ol1z9x

I noticed this visual issue, that row is a little off, which we would need to see what happened there.. And at first try, smooth scrolling did not work well for some reason, which we also need to investigate.

HaveSpacesuit commented 2 years ago

@veekeys You have a nice work around on the linked sandbox, but it does not work with a virtualized table. If we did add a prop to the table, it would be nice to have it work in the virtualized table as well.

veekeys commented 2 years ago

@HaveSpacesuit yes, virtualized list cannot work with native scrollToView because the item might be not rendered. We do have PR #539 with the support of scrollToIndex, though. We are currently refactoring Combobox to figure last bugs while using virtual scroll and then this should be available. Just will need additional prop to the table, I guess.

HaveSpacesuit commented 2 years ago

Oh, that's great! I this this feature is going to be a must-have for our team to migrate to the iTwin table.

HaveSpacesuit commented 2 years ago

@veekeys Now that PR #539 is complete, it is possible to add a scrollToIndex (or scrollToItem?: T) prop to the table. I've tested it out locally, and it works fine with virtualization.

Would this be a good fit to add to the table? I imagine it will get more hairy when using paging or trying to support non-virtualized tables.

Maybe to keep it simple, the table could only support scroll to index with virtualization. I imagine most users who want this functionality want it because they have a large table, and they are already either using virtualization or paging. The consumer can control the page for paged tables. For non-virtualized tables, there is the work around you give in the code sandbox.

Like I mentioned above, my team really needs a feature like this for situations like sharing a link to an item, or refreshing the data after modifying a selected item.

veekeys commented 2 years ago

@HaveSpacesuit Hey, good you followed it! If you have already tried it out, would you be interested in contributing again? I have few tasks to finish before I would move to this one, so if you just have a PR with the basic implementation created, we could move faster with this. The only thing need to think is about the API part. If we want to attach this only to tables with enabled virtualization, or would we think about adding this support to all tables (still not sure about the ones with paging.. seems like there this should not be available).

HaveSpacesuit commented 2 years ago

@veekeys I've started a conversation about this in PR #689

veekeys commented 2 years ago

This is now released with 1.42.0

Thanks @HaveSpacesuit for contribution!!