Skip to content

Conversation

@gorhom
Copy link

@gorhom gorhom commented Jul 30, 2020

Motivation

Hi @rgommezz & @djorkaeffalexandre 👋,

first i want to thank you for creating this amazing library, it address one of the main issues with reanimated-bottom-sheet 🔥

However, i notice that you embedded scrollable components in ScrollBottomSheet component, which makes it difficult to customise and extend this library functionality.

Luckily, I am working on a project that needed this library and i had spent time on refactoring it and get scrollable components extracted and did some clean up.

Changelog

  • Extracted FlatList, SectionList & ScrollView
  • Implemented ScrollBottomSheetContext to allow scrollable components to access animated values, refs and other values from ScrollBottomSheet
  • Removed Expo and other libraries from example

Todo

  • Update documentation
  • Add FlatList horizontal example back
  • Allow scrollable to hold multiple refs

gorhom added 4 commits July 22, 2020 22:24
* refactor: remove expo WIP

* refactor: removed expo and simplifed example
* chore: added redash

* chore: extract flatlist & scrollview from scrollbottomsheet

* chore: added sectionlist
* chore: updated examples

* chore: reset root screen
@rgommezz
Copy link
Owner

rgommezz commented Jul 31, 2020

Hi @gorhom, kindly appreciate your words.

However, i notice that you embedded scrollable components in ScrollBottomSheet component, which makes it difficult to customise and extend this library functionality.

That's indeed the sole purpose of this library, to address scrollable components, hence the word "scrollable" in the name. It's meant to be minimalistic. That was my number one goal when I developed it.

Now, this is almost a complete re-write of the library from scratch. I don't know how you'd expect me to review and merge such a drastic change.

Also, you have removed the Expo example, which is key for people to quickly try out the library without all the extra hassle of dealing with native dependencies.

Feel free to use your own fork in your projects. I am happy you were able to extend it for other use cases but unfortunately I can't merge these changes upstream.

For the future, I'd recommend you check first with the library owner when you attempt to carry out some work of these dimensions, just by opening an exploratory issue, so that at least we can have a discussion and plan a potential implementation together (see last point of the contributing guidelines). It's sort of standard and will avoid wasting any time down the line.

I am sorry and I hope you understand.

@gorhom
Copy link
Author

gorhom commented Jul 31, 2020

@rgommezz , totally understood , thanks for your feedback 👍

@gorhom gorhom closed this Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants