search

nuclearpasta / react-native-drax

A drag-and-drop system for React Native
MIT License
554 stars 69 forks source link

Forward scrollView ref #107

Closed haswalt closed 3 years ago

haswalt commented 3 years ago

Based on the work by @chrisdrackett here https://github.com/nuclearpasta/react-native-drax/pull/85 adds support to forward ref to the scrollview.

lafiosca commented 3 years ago

Thank you for the PR. I am not sure how soon I'll get a chance to look at it in detail. Maybe I can review this and #85 at the same time when I'm able.

haswalt commented 3 years ago

@lafiosca i have been considering what you said here: https://github.com/nuclearpasta/react-native-drax/pull/85#issuecomment-829611652 but I can’t really think of a good reason why you would want to access the DraxScrollView over the child ScrollView.

Personally I prefer the idea that DraxScrollView (and DraxList) are drop in replacements for their standard react-native components (ScrollView etc).

haswalt commented 3 years ago

On a side note, we’re due to rely quite heavily on this library for one of our up coming products, we’d be happy to help maintain or sponsor you if that helps

lafiosca commented 3 years ago

@haswalt I'm open to the idea of sharing the maintenance load. Sponsorships are appreciated, but the fact is I have not had much availability over the past year to take care of this library as deeply or often as I would like to, and just this week I started a new job which is not focused on React Native. I try to answer questions and fix bugs where I am able, but if I had the time and energy I would like to work on a new iteration of the entire library, incorporating lessons learned since it was first written and researching how things have changed in the subsequent RN releases.

lafiosca commented 3 years ago

Thank you again. One note: this diff has a lot of cosmetic changes that are unrelated to the logic change and make it more difficult for me to quickly review this PR at a glance. If those could be removed to clean up the diff, it would help me out a lot.

haswalt commented 3 years ago

@lafiosca sorry about that. My editor wanted to use some defaults. I’ll try and clear up and add an editor config file so it doesn’t happen to others in this pr. won’t be able to look at it until Monday though

oktaysenkan commented 3 years ago

Any update?

SandeshVakale commented 3 years ago

@lafiosca any update on this topic.

Can you please review this PR and Merge it?

haswalt commented 3 years ago

@oktaysenkan @SandeshVakale @lafiosca I've not had a chance to revisit this PR to fix the issues with the formatting.

I'll be looking at this in the week to provide a cleaned up PR hopefully ready for @lafiosca to merge

lafiosca commented 3 years ago

Thanks @haswalt, that will help. Also I may have found a good approach for working prettier into the project (as suggested in #73 and #74) which could make these situations simpler in the future, but I am not sure when I'll get around to working on it.

SandeshVakale commented 3 years ago

Hello @haswalt @lafiosca, Sorry to ask you again for updates. My project is blocked because of this issue.

Is there anything I can help ?

haswalt commented 3 years ago

If it helps unblock your project you can use our fork with this change. There is a npm package published to GitHub packages for it.

From: SandeshVakale @.> Sent: Thursday, October 21, 2021 10:17:26 AM To: nuclearpasta/react-native-drax @.> Cc: Harry Walter @.>; Mention @.> Subject: Re: [nuclearpasta/react-native-drax] Forward scrollView ref (#107)

Hello @haswalthttps://github.com/haswalt @lafioscahttps://github.com/lafiosca, Sorry to ask you again for updates. My project is blocked because of this issue.

Is there anything I can help ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/nuclearpasta/react-native-drax/pull/107#issuecomment-948418939, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AACSVHFKPQI3RIV3YS2UQ6TUH7LCNANCNFSM5CBMO32A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

lafiosca commented 3 years ago

@SandeshVakale If you can't wait, I suggest using a local patch or @haswalt's fork. Otherwise I will try to get to this functionality when I am able, regardless of PR cleanup.

SandeshVakale commented 3 years ago

Hi again, I need help to install this library from github.

When I installs @lafiosca npm package, it creates build folder inside library in node_modules.

But when I install @haswalt 's library like this npm i --save BlueBambooStudios/react-native-drax#feature/forward-scroll-ref It does not creates build folder.

I can also see scripts inside package.json that should create build folder I guess. But I don't get build folder in my node_modules library location.

Am I doing anything wrong?

Thanks in advanced.

haswalt commented 3 years ago

You need to install the package not using fit.

From: SandeshVakale @.> Sent: Thursday, October 21, 2021 10:11:23 PM To: nuclearpasta/react-native-drax @.> Cc: Harry Walter @.>; Mention @.> Subject: Re: [nuclearpasta/react-native-drax] Forward scrollView ref (#107)

Hi again, I need help to install this library from github.

When I installs @lafioscahttps://github.com/lafiosca npm package, it creates build folder inside library in node_modules.

But when I install @haswalthttps://github.com/haswalt 's library like this npm i --save BlueBambooStudios/react-native-drax#feature/forward-scroll-ref It does not creates build folder.

I can also see scripts inside package.json that should create build folder I guess. But I don't get build folder in my node_modules library location.

Am I doing anything wrong?

Thanks in advanced.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/nuclearpasta/react-native-drax/pull/107#issuecomment-949006269, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AACSVHBN3ZRIHPSLNVUUKA3UIB6XXANCNFSM5CBMO32A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

haswalt commented 3 years ago

Here is the package info: https://github.com/BlueBambooStudios/react-native-drax/packages/939637

GitHub packages needs a tiny bit of setup to tell your project to use their servers instead of npm for that package: https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-npm-registry#installing-a-package

From: SandeshVakale @.> Sent: Thursday, October 21, 2021 10:11:23 PM To: nuclearpasta/react-native-drax @.> Cc: Harry Walter @.>; Mention @.> Subject: Re: [nuclearpasta/react-native-drax] Forward scrollView ref (#107)

Hi again, I need help to install this library from github.

When I installs @lafioscahttps://github.com/lafiosca npm package, it creates build folder inside library in node_modules.

But when I install @haswalthttps://github.com/haswalt 's library like this npm i --save BlueBambooStudios/react-native-drax#feature/forward-scroll-ref It does not creates build folder.

I can also see scripts inside package.json that should create build folder I guess. But I don't get build folder in my node_modules library location.

Am I doing anything wrong?

Thanks in advanced.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/nuclearpasta/react-native-drax/pull/107#issuecomment-949006269, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AACSVHBN3ZRIHPSLNVUUKA3UIB6XXANCNFSM5CBMO32A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

SandeshVakale commented 3 years ago

Hello @haswalt Thanks for your help,

I managed to install your package. thanks again

this link also helped me https://stackoverflow.com/questions/58919401/installing-packages-from-github-npm-registry-auth-error-401

lafiosca commented 3 years ago

Thank you for the effort @haswalt

lafiosca commented 3 years ago

Drax 0.9.1 has been released, which forwards refs for DraxScrollView #117 and DraxList #118 imperative calls, as well as fixing the handling of style on DraxScrollView #119

I apologize for how long this took!