feat(components-native): Integrate InputText with ContentOverlay

[ZakaryH]

…rd interactions

Motivations

when using an InputText inside a ContentOverlay (powered by the gorhom bottomsheet lib) there's extra complexity from the fact that the bottomsheet moves around between snap points, can scroll and there will be a virtual keyboard present when focusing an InputText. essentially it's not simple, and to help orchestrate all this - the library ships with its own InputText component.

we'd prefer not to use that, and instead have our own InputText that people reach for, be compatible with all this. to that end, we've integrated the relevant code from the other InputText as per the docs

https://gorhom.dev/react-native-bottom-sheet/keyboard-handling

large screens
the other change in here is to make the ContentOverlay have a different appearance on large devices where it looks a bit comical to have a bottom sheet with so little content spanning the entire width.

Changes

Added

added a new Provider, ContentOverlayProvider - which does a couple things:

• abstracts the underlying library so that we're not directly using the gorhom bottomsheetProvider
• signals to InputText that if it's within a ContentOverlay, to use some additional handlers to work nicely with keyboard

Changed

had to update the logic for adjustToContent height and the way we assign snap points a little. this is because there's an issue with the library that results in really not great behavior when dismissing the keyboard. essentially the "restore" just doesn't work and you can have a floating, excessively large BottomSheet left with no keyboard showing. one fix for this is to simply use a 100% snap point. it's important that it's 100% specifically.

Deprecated

Removed

Fixed

Security

Testing

you should try this on device, for iOS and Android ideally. simulators are the next best option. trying it in storybook is not sufficient.

I'll link my testing branch, but if you want to do it yourself just use a prerelease of this branch, and then wrap the app in a ContentOverlayProvider. then set up a ContentOverlay somewhere to interact with.

put an InputText, InputNumber, InputDate and maybe a couple others inside a ContentOverlay. when you focus them the keyboard should correctly still be visible and usable. dismissing the keyboard should work as expected, with the COntentOverlay going back to its original dimensions.

additional props like fullScreen, adjustToContentHeight, isDraggable, scrollEnabled should all still work as expected, both with and without inputs present.

large screens
for this change you'll need a tablet. if you don't have one, a simulator can work.

all that changes is that it should be a centered, not full width ContentOverlay now. everything else, including keyboard stuff should be unchanged.
Changes can be
tested via Pre-release

---

In Atlantis we use Github's built in pull request reviews.

[cloudflare-workers-and-pages]

Deploying atlantis with    Cloudflare Pages

Latest commit:	81d2777
Status:	✅  Deploy successful!
Preview URL:	https://8e4dad03.atlantis.pages.dev
Branch Preview URL:	https://job-144159-the-inputs-within.atlantis.pages.dev

View logs

[github-actions]

Published Pre-release for 8b5972a with versions:

  - @jobber/components-native@0.89.5-JOB-144159-8b5972a.110+8b5972a40

To install the new version(s) for Mobile run:

npm install @jobber/components-native@0.89.5-JOB-144159-8b5972a.110+8b5972a40

[jdeichert]

I think this comment doesn't make as much sense anymore, where it is at least. Before it was directly above adjustToContentHeight so it was more clear what it was talking about.

This ternary is a tiny 🤏 bit difficult to comprehend.. a comment about why it needs to work this way might be worth it?

[jdeichert]

Likewise, if you feel like a util function with early returns is more clear to read (and easier to add comments to), that works for me too!

[ZakaryH]

that's fair on both counts. the comment can be massaged and moved, or if just doesn't help anymore I can get rid of it.

as for the ternary. I agree. I will make a named function to try and make it more readable/understandable.

while our logic in here is really questionable, there are ways we can at least make it a bit easier to follow.

[ZakaryH]

moved it into a function and I had the comment around an extra condition

  // When this is enabled, it prevents the modal from being dragged to fullscreen. This is solely necessary
  // to match the behaviour of the previous library. Again, revisit in a future refactor.
  // Ideally the initial height of the modal (full vs content height) is a separate decision from the ability to drag it.
  if (adjustToContentHeight) {
    return false;
  }

but I remembered something I came across during testing. the new BottomSheet doesn't let you turn off dragging in one direction only 😬

by that I mean the old one, with its code to handle adjustToContentHeight - you wouldn't be able to drag it up to make it taller, but you could still pan/swipe down to close it.

the new one, if you turn off drag it can't be closed with swipe/pan. personally I don't even understand why adjustToContentHeight affects dragging at all. they are completely different ideas. to me it feels pretty bad if you can't close it with a swipe AND we have the dismiss button hidden which does match the old one.

we can run this by design, but I vote that we just let people drag regardless of adjustToContentHeight. I see no downside to allowing someone to extend it. maybe there's more white space if you go out of your way to drag it up, but that's such a minor thing compared to not being able to close it with a pan gesture.

this does also make the dismiss button logic weird, but it's already bizarre.

[jdeichert]

Sounds good - I'll approve and merge this down to the main PR and we can followup with design this week 👍

[jdeichert]

Nice 👍

[jdeichert]

Glad to see the focus and blur logic isn't too bad here! Obviously not ideal, but feels better than being forced to ship separate input sub components with ContentOverlay 👌

[jdeichert]

Nice work!