naoufal / react-native-progress-hud

A clean and lightweight progress HUD for your React Native app.
https://www.npmjs.com/package/react-native-progress-hud
258 stars 54 forks source link

Context warnings #6

Closed grabbou closed 8 years ago

grabbou commented 9 years ago

I am using this throughout my apps as well, but since I am using flux implementation and props driven UI manipulation - I do not use context at all (and actually somehow can't without hacks since I have ES6 classes). It would be cool to improve the current implementation to make mixin a complete opt-in, as it seems to work without it with no problems.

Ps. Since activity view already have one maintainer - gonna reserve this one if there's such need in the future :dancer:

naoufal commented 9 years ago

@grabbou Ran into this problem myself when using ES6 classes. In the web implementation of the Progress HUD I use higher order components to get around using mixins.

Perhaps we could use that over here.

Thoughts?

naoufal commented 9 years ago

While we're at it, we should also use the built-in Modal component.

marcshilling commented 9 years ago

+1 for higher order components!

grabbou commented 9 years ago

@naoufal https://github.com/naoufal/react-native-progress-hud/blob/master/src/index.js#L54

Can we just make it optional? Also, since dismissProgressHUD on context is optional, we should add extra check for it here https://github.com/naoufal/react-native-progress-hud/blob/master/src/index.js#L107

naoufal commented 9 years ago

@grabbou We'll also need to update the documentation since remove context will break the isDismissible prop (the way it's currently documented).

What I was thinking of doing is actually removing the ProgressHUD Component all together and only providing a Higher Order Component that would add the markup to the root of your app. Something like this:

export default ProgressHUDWrapper(YourApp);

ProgressHUDWrapper (name is debatable) would set showProgressHUD() and dismissProgressHUD() as props on YourApp.

The showProgressHUD function would take an options argument to optionally set:

Like so:

this.props.showProgressHUD({
  overlayColor: "rgba(0, 0, 0, 0.3)",
  color: "#000",
  isDismissible: true,
  dismissCallback: () => {}
});

Being optional, showProgressHUD() could of course be called without the options.

@grabbou @marcshilling What are your thoughts on this?

grabbou commented 8 years ago

I am unfortunately no longer using this module in my apps so I am afraid I won't be able to help! I decided to wrap my custom one with https://github.com/oblador/react-native-progress and reached the same functionality w/o it :)

naoufal commented 8 years ago

No worries. In that case, I'm going to close this for now. :smile: