feat: add a ios native module to calculate inset top and bottom

[seyedmostafahasani]

I used react-native-safe-area-context to handle it.

[calintamas]

hey @seyedmostafahasani, thanks again!

I'm just thinking: could we do it in some way that doesn't involve adding a 3rd party dependency? Adding react-native-safe-area-context brings with itself a couple of complexities for the end-user of the library, like installing the native pods, wrapping the <Toast /> in the <SafeAreaProvider />, dep version incompatibilities, etc

I suggest either:

• read the actual initial safe area values ourselves through a native module, written as part of toast message lib (so we don't depend on 3rd parties)
• come up with some good-enough topOffset and bottomOffset values for different iOS versions. Could be done in a trial-and-error fashion, on different simulators

wdyt?

[seyedmostafahasani]

hey @seyedmostafahasani, thanks again!

I'm just thinking: could we do it in some way that doesn't involve adding a 3rd party dependency? Adding react-native-safe-area-context brings with itself a couple of complexities for the end-user of the library, like installing the native pods, wrapping the <Toast /> in the <SafeAreaProvider />, dep version incompatibilities, etc

I suggest either:

• read the actual initial safe area values ourselves through a native module, written as part of toast message lib (so we don't depend on 3rd parties)
• come up with some good-enough topOffset and bottomOffset values for different iOS versions. Could be done in a trial-and-error fashion, on different simulators

wdyt?

Hey Calin,
I completely agree with you about 3rd party dependency. I will try to fix it with one of the above methods.

[calintamas]

@seyedmostafahasani looking good, thanks! I'll run a few tests in a demo app that I use and then we can merge. Will probably require a major bump, since it's a breaking change for installation.

[seyedmostafahasani]

@seyedmostafahasani looking good, thanks! I'll run a few tests in a demo app that I use and then we can merge. Will probably require a major bump, since it's a breaking change for installation.

Anytime! I agree with you about a major bump. Great!
I tested it on different devices (iOS and Android), and it works correctly. Please check it out.

[seyedmostafahasani]

@calintamas Did you have time to test my PR?

[calintamas]

@calintamas Did you have time to test my PR?

hey, I did not manage to take a look yet. will try today

[calintamas]

@seyedmostafahasani Took a quick look, and everything looks fine! I just need some time to prepare the new readme for v3, with release notes migration docs, etc.

I published the change as v3.0.0-beta.1, if you need it in the meantime:

yarn add [email protected]

[seyedmostafahasani]

@seyedmostafahasani Took a quick look, and everything looks fine! I just need some time to prepare the new readme for v3, with release notes migration docs, etc.

I published the change as v3.0.0-beta.1, if you need it in the meantime:

yarn add [email protected]

I can help you if you need anything.

[seyedmostafahasani]

Hey Calin,
I hope you are well.

Have you had enough time to release a new version of the library?

[calintamas]

Ah, not really :) Winter vacation and all that, I'll get back to it soon

[seyedmostafahasani]

@calintamas Hey Man,
I can help you to release a new version of the library if you need anything.

[calintamas]

Does the native module work fine with Expo, or we need ajustments? I wanted to have this checked, but really had no time to do it.

[seyedmostafahasani]

Does the native module work fine with Expo, or we need ajustments? I wanted to have this checked, but really had no time to do it.

Hey Man,
I think it works correctly but I will test it on a project with Expo then I will inform you about it.

[seyedmostafahasani]

Does the native module work fine with Expo, or we need ajustments? I wanted to have this checked, but really had no time to do it.

Hey Man, I think it works correctly but I will test it on a project with Expo then I will inform you about it.

Hey @calintamas
I hope you are well.
I tested this native module on Expo and it worked correctly, but it doesn't work on "Expo Go" as it requires custom native code. On "Expo Go" the toast message uses default number.
If you agree we can release a new version of the library.

[seyedmostafahasani]

If you need anything let me know. I'm available 🤝

[calintamas]

Hey! I kept thinking if it's worth having a native module (only) for this after all...

I'm leaning towards the other solution I proposed back then -- some good-enough (hardcoded) defaults, with the already existing topOffset and bottomOffset props to overwrite them. It's simple and it worked fine as a solution until now, so why add more complexity?

A native module means: (1) more maintenance work down the road (with the new RN architecture), (2) potential issues for Expo users (which I don't have time to take care of, since I'm not actively using Expo).

I probably should have expressed my concerns earlier. I don't want to dismiss the work you did in any way, I just prefer solutions that are simpler to maintain in the long-term. Thanks for understanding!

[seyedmostafahasani]

Hey! I kept thinking if it's worth having a native module (only) for this after all...

I'm leaning towards the other solution I proposed back then -- some good-enough (hardcoded) defaults, with the already existing topOffset and bottomOffset props to overwrite them. It's simple and it worked fine as a solution until now, so why add more complexity?

A native module means: (1) more maintenance work down the road (with the new RN architecture), (2) potential issues for Expo users (which I don't have time to take care of, since I'm not actively using Expo).

I probably should have expressed my concerns earlier. I don't want to dismiss the work you did in any way, I just prefer solutions that are simpler to maintain in the long-term. Thanks for understanding!

Thank you for your attention. I am available to manage the library and address any issues related to this native module. Please don’t worry about it. We can release a new version with these changes to the native module. Additionally, many of my friends have used the beta version in various projects, and none of them have encountered any issues with native modules.

[seyedmostafahasani]

We also haven't removed the default variables topOffset and bottomOffset. If the native module doesn't function correctly, the library returns their default values. in a nutshell, don't worry about these changes.

[calintamas]

I understand your point. What I mean is that I don't think the native module is really necessary in this case (along with the reasons mentioned above, it brings a breaking change which I prefer to avoid).

If anyone needs custom values, it's relatively easy to do even right now:

import { Toast } from 'react-native-toast-message'

const App = () => {
  const insets = useSafeAreaInsets()
  
  return (
    <Toast
      // ...
      topOffset={insets.top}
      bottomOffset={insets.bottom}
    />
  )
}

I'd rather have some better default values (trading correctness for simplicity). It's a difference in the approach, nothing else.

[seyedmostafahasani]

I understand your point. What I mean is that I don't think the native module is really necessary in this case (along with the reasons mentioned above, it brings a breaking change which I prefer to avoid).

If anyone needs custom values, it's relatively easy to do even right now:

import { Toast } from 'react-native-toast-message'

const App = () => {
  const insets = useSafeAreaInsets()
  
  return (
    <Toast
      // ...
      topOffset={insets.top}
      bottomOffset={insets.bottom}
    />
  )
}

I'd rather have some better default values (trading correctness for simplicity). It's a difference in the approach, nothing else.

I understand your point. This way is good, but we can do it easier for all users with these changes. They don't need to install 3rd-party library.