Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

react-native for web support #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

react-native for web support #31

wants to merge 2 commits into from

Conversation

alexkendall
Copy link

  • Adds support to snap to items on react-native for web
  • Integrates hooks logic to get current index while scrolling events take place
  • Snaps to items automatically after a 500ms timeout on web
  • Only runs additional hooks code on web to keep performance the same on mobile

@alexkendall alexkendall changed the title Adds react-native for web support react-native for web support Jun 9, 2022
@erksch
Copy link
Owner

erksch commented Jun 13, 2022

@alexkendall Thanks for contributing!

I'll check out the changes as soon as possible.

erksch
erksch requested changes Jun 13, 2022
View reviewed changes
src/WheelPicker.tsx Outdated
}, [scrollIndex])

const handleScroll = (event: {
nativeEvent: {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the type Animated.EventConfig<NativeScrollEvent>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be updated now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also update the other occurence

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

src/WheelPicker.tsx Outdated
@@ -88,6 +93,47 @@ const WheelPicker: React.FC<Props> = ({
}
};

useEffect(() => {
if (Platform.OS === "web") {
const SCROLL_COOLDOWN_MILISECONDS = 100
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo MILISECONDS => MILLISECONDS

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be updated now.

src/WheelPicker.tsx Outdated
if (difference > SCROLL_DID_STOP_TIMEOUT) {
flatList.current?.scrollToIndex({ index: scrollIndex, animated: true })
}
}, SCROLL_COOLDOWN_MILISECONDS)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo MILISECONDS => MILLISECONDS

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be updated now.

src/WheelPicker.tsx Outdated
@@ -54,8 +55,12 @@ const WheelPicker: React.FC<Props> = ({
containerProps = {},
}) => {
const [scrollY] = useState(new Animated.Value(0));
const flatList = useRef<any>(null)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have a type

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be called flatListRef

Copy link
Author

@alexkendall alexkendall Jun 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use flatListRef

src/WheelPicker.tsx Outdated

useEffect(() => {
if (Platform.OS === "web") {
if (onChange) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onChange is not nullable

src/WheelPicker.tsx Outdated
}
}) => {
if (Platform.OS === "web") {
const positionY = event?.nativeEvent?.contentOffset?.y ?? 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the ?, these types are not nullable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ?'s have been removed.

src/WheelPicker.tsx Outdated
@@ -54,8 +55,12 @@ const WheelPicker: React.FC<Props> = ({
containerProps = {},
}) => {
const [scrollY] = useState(new Animated.Value(0));
const flatList = useRef<any>(null)

const lastScrollTimestamp = useRef(new Date().getTime())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing semicolon, please format the code with yarn prettier --write src

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reformatted with the latest commit.

src/WheelPicker.tsx Outdated
contentSize: { height: number, width: number },
layoutMeasurement: { height: number, width: number }
}
}) => handleScroll(event) },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code formatting

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reformatted with the latest commit.

@alexkendall alexkendall force-pushed the master branch 2 times, most recently from 9c7b8f1 to 6e18de8 Compare August 23, 2022 14:40
@alexkendall
Adds support for react-native-web.
df6b5ff 
Snaps to item automatically on react-native-web with usage of cooldown.
@alexkendall
Copy link
Author

@erksch Went ahead and rebased and squashed the commits to clean up the history and rebased with master. I had to make a few changes to the scroll event to get the typescript to build without errors, and adding the Animated types did not work. It should compile without errors now.

@TNAJanssen
Copy link

@erksch is it possible to get this merged?

@Saikedo
Copy link

Saikedo commented Nov 15, 2023

Is there any hope to get this merged any time soon? Thank you.

@jbagaresgaray
Copy link

Hello.... Any news on this PR? When can we merge this?

@Jaza
Copy link

Jaza commented Aug 27, 2024

Thanks @alexkendall for working on this. I'm hoping to use react-native-wheely in an Expo Web enabled project. @erksch any chance we can get this merged and into a release?

@Jaza Jaza mentioned this pull request Aug 27, 2024
Open
@Jaza
Copy link

Jaza commented Dec 27, 2024

@erksch I tested this in Expo Web, and it works. Would be great to get this merged and into a release soon.

If anyone else wants to try out this change right now, here's what I did (starting in Expo project's base directory):

npm install -g yarn
npm install --save "https://github.com/alexkendall/react-native-wheely/tarball/master"
cd node_modules/react-native-wheely
yarn install
npm run build
cd ../..
npx expo install

A bit annoying to install, but it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erksch erksch erksch requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexkendall @erksch @TNAJanssen @Saikedo @jbagaresgaray @Jaza