react-native-screens icon indicating copy to clipboard operation
react-native-screens copied to clipboard

Published 20 hours ago • verified publisher icon software-mansion

Add API for imperative actions on RNSSearchBar

Open egegunes opened this issue 1 year ago • 3 comments

Description

This changes allow users to manipulate the RNSSearchBar. For use cases please see discussions/1323.

Changes

  • Exported 5 new methods from RNSSearchBarManager in ios/RNSSearchBar.mm.
    1. focus
    2. blur
    3. toggleCancelButton
    4. clearText
  • Also added the same methods to SearchBar in src/index.native.tsx to call native ones
  • Added ref key to SearchBarProps in src/types.tsx

Test code and steps to reproduce

Added buttons to Example app to test each action:

  1. Focus
  2. Blur
  3. ToggleCancelButton
  4. ClearText

Checklist

  • [x] Included code example that can be used to test this change
  • [x] Updated TS types
  • [x] Updated documentation:
    • [x] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
    • [x] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
    • [x] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
    • [x] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
  • [ ] Ensured that CI passes

egegunes avatar Jul 09 '22 19:07 egegunes

@kkafar I fixed the conflict and updated the docs, please let me know if it needs any improvements. We're using this in our internal builds for some time and it looks good so far.

egegunes avatar Aug 14 '22 15:08 egegunes

Hi @egegunes! Thank you for your contribution 🎉 It looks good, although I believe these changes may have some issues on Fabric, but it needs to be tested.

I'll do it & let you know in couple of days.

kkafar avatar Aug 17 '22 07:08 kkafar

Sorry for the delay, I'm back on it.

Your PR looks good and would be ready to merge, but I indeed encountered some issues while implementing the code for Fabric (can't get to codegened native commands w/o registering the RNSSearchBar component twice, you can reproduce on #1610) so I won't merge it for now, as I want to have keep the API identical for both architectures. However thank you for your contribution! I'll have it merged as soon as I tackle with Fabric implementation.

kkafar avatar Oct 11 '22 13:10 kkafar

This is awesome @egegunes. I'd love to contribute but my knowledge in objc is quite limited. Could you have a look at this discussion and help me assess if this is a low-hanging fruit that could be included with this api changes? I'd love to help out!! Thanks!

https://github.com/software-mansion/react-native-screens/discussions/1617

felixaa avatar Dec 02 '22 11:12 felixaa