Migrate Hint component to TypeScript

[saulve]

Description

This PR migrates Hint component and its example screen to TypeScript.

Changelog

Removes manual typings in Hint.d.ts

[Inbal-Tish]

Why the change of type to () => void?

[saulve]

because onRequestClose expects a function with () => void signature, however onBackgroundPress has (event: GestureResponderEvent) => void. Therefore I'm adding a type assertion here.

[M-i-k-e-l]

This looks like a bug, because the user that sends the onBackgroundPress can expect to receive an event and may rely on it. I think you need to change the prop to:

onBackgroundPress?: (event?: GestureResponderEvent) => void;

(and remove the as () => void)

[saulve]

I think it's actually the other way around.

ModalProps ( in src/components/modal/index.tsx) is defined:

/**
  * allow dismissing a modal when clicking on its background
  */
onBackgroundPress?: (event: GestureResponderEvent) => void;

Whereas onRequestClose is defined:

/**
  * The `onRequestClose` prop allows passing a function that will be called once the modal has been dismissed.
  * _On the Android platform, this is a required function._
  */
onRequestClose?: () => void;

IMO it's important to keep the onBackgroundPress definition as is because if the user uses onBackgroundPress prop then event should always be defined (as you've described). onRequestClose, however does not expect a function argument, so we can override it. I think the only reason we use onRequestClose is because it's a required function for android (onBackgroundPress is what should actually dismiss the modal) . Alternatively, maybe we could even have something like onRequestClose={this.noop} (noop = () => {})?

[M-i-k-e-l]

Maybe we should take this offline.

[M-i-k-e-l]

Nice work and thank you!

I've left some comments, you also need to update generatedTypes (it's a little duplication of src/index :/ ).
There are some things that I'd like to see changed, but I'm not sure how easy they are, feel free to try:

1. Changing the refs to something other than any.
2. Reducing the number of interfaces, I've suggested one, you can open a ticket on this if you don't see other improvements - this might require some infra work.

[M-i-k-e-l]

This looks like a bug, because the user that sends the onBackgroundPress can expect to receive an event and may rely on it. I think you need to change the prop to:

onBackgroundPress?: (event?: GestureResponderEvent) => void;

(and remove the as () => void)

[M-i-k-e-l]

Oh, another thing, the branch name should be something a little more specific (maybe hint-migraion-to-ts), for the next PR :)

[saulve]

@M-i-k-e-l thanks for your time reviewing this. Is there anything else left for me to address?

[M-i-k-e-l]

Nice work, I think there are only two things left, maybe we should talk about the onBackgroundPress