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.

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

Plans for this library? #1

Closed
nandorojo opened this issue Oct 4, 2021 · 12 comments
Closed

Plans for this library? #1

nandorojo opened this issue Oct 4, 2021 · 12 comments
Labels
question Further information is requested

Comments

@nandorojo
Copy link
Contributor

nandorojo commented Oct 4, 2021

Hey there!

Wow – this looks amazing. I've been using Rainbow's charts and just stumbled upon this. The docs and composability look super promising. I also love that you have candle stick charts (and plans for others).

I have a more general question - is this a library that you plan to maintain going forward? It looks really solid so I figured the answer is yes, but just wanted to confirm that it plans to get updates over time, etc.

Also, do you think web support might be in the cards? I'm not entirely sure how Reanimated 2's support is for web SVG (I think there may be an issue with this). But if that's something you'd like to support, I could help test it, and maybe open a related issue on the Reanimated library if needed.

For context, I maintain a number of React Native (+Web) libraries, such as Moti and Dripsy, so I'm excited to see another cool project based on Reanimated 2.

@jxom
Copy link
Contributor

jxom commented Oct 5, 2021

Hey @nandorojo

Thanks for the kind words! We are definitely planning to maintain this library going forward, as it will be our main charting library for CoinJar. We are only using the charts in a small part of our app at the moment, but once we start to adopt the library across our apps, we will be making further enhancements and adjustments when required.

CoinJar also uses React Native Web, so web support is definitely on the cards. Reanimated 2 is a bit hit and miss on the web right now, so when the time comes, we may have to fork off for some functionalities for the web (though, haven't thought much about this yet).

@jxom jxom added the question Further information is requested label Oct 5, 2021
@jxom jxom pinned this issue Oct 5, 2021
@jxom
Copy link
Contributor

jxom commented Oct 5, 2021

Will pin this so it's not forgotten about.

@nandorojo
Copy link
Contributor Author

Got it, good to know.

FWIW, the next version of Reanimated comes with much better web support. There was an issue with RNW that caused spring transitions to flicker, but they got around it in the upcoming release. I'm using RNW as well, with Moti (and thus Reanimated 2) in production with this fix, and it's actually performed quite well.

That's great to hear though. I'd love to try this out on web. If it doesn't work, I'll try to come up with a reproducible test case.

One more question: would you consider using expo-haptics instead? This would play more nicely with managed expo apps. Expo can still use any Native code with a custom dev client, however it will break on Expo Snack and the normal Expo client.

Thanks again!

@jxom
Copy link
Contributor

jxom commented Oct 5, 2021

One more question: would you consider using expo-haptics instead?

Hmm, I would, but I guess the dependency would be that bare RN apps would have to integrate with react-native-unimodules. But maybe this is okay since it may be better to have interop across Expo & bare RN, and it's not too difficult to install unimodules anyway.

@andreialecu
Copy link
Contributor

Would be great to have an option to not use expo-haptics.

We're already using react-native-haptic-feedback so having to bring in unimodules and expo-haptics as well kind of increases the maintenance overhead.

A possible option would be to have support for both, and detect which one is installed.

Optional peer dependencies are supported by all package managers via peerDependenciesMeta (ref: https://docs.npmjs.com/cli/v7/configuring-npm/package-json#peerdependenciesmeta). It would be easy to detect which peer is installed and have a code path that uses one haptic module or the other.

@nandorojo
Copy link
Contributor Author

nandorojo commented Oct 6, 2021

Stream Chat achieves this by having a separate export for expo users: https://github.com/GetStream/stream-chat-react-native/blob/develop/package/expo-package/src/index.js#L169

They essentially have a headless haptics object which you can mutate from anywhere. I’d be fine with this API, it would only be a matter of exposing it.

This library could export a Haptics object which you can mutate directly with your own implementation.

@andreialecu
Copy link
Contributor

andreialecu commented Oct 6, 2021

My proposal was to not have a separate export in order to make this transparent to users of the library.

This could be achieved by adding optional peer dependencies on both expo-haptics and react-native-haptic-feedback.

It's possible to detect one or the other via something like:

function isModuleAvailable(module: string) {
  try {
    require.resolve(module);
    return true;
  } catch {
    return false;
  }
}

const HapticsType: 'expo' | 'rn' = isModuleAvailable('expo-haptics') ? 'expo' : 'rn';

@nandorojo
Copy link
Contributor Author

Yeah I think that would work

@jxom
Copy link
Contributor

jxom commented Oct 6, 2021

Yeah, that could work too. Great idea. Thanks @andreialecu!

@nandorojo
Copy link
Contributor Author

Another nice thing about @andreialecu’s approach is that you can log a warning if neither package is installed without breaking the app.

@jxom
Copy link
Contributor

jxom commented Oct 7, 2021

Have opened an issue for this on #3.

If anyone is wanting to contribute and tackle this, let me know. Otherwise, I'll investigate when I get more time.

@jxom
Copy link
Contributor

jxom commented Oct 7, 2021

Going to close this one for now as we've addressed the #3 and hopefully #2.

@jxom jxom closed this as completed Oct 7, 2021
@jxom jxom unpinned this issue Oct 7, 2021
simonitfi pushed a commit to simonitfi/react-native-wagmi-charts that referenced this issue Jan 23, 2024
…th-data-chart-option-implementation

SPD-1399 switching between smoothed data and original based on isActive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants