feat: prepare for new search modal #11033
Conversation
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
f87aeaa
to
a810d3e
Compare
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
93382c4
to
4b5a128
Compare
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
4b5a128
to
e14d934
Compare
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
e14d934
to
da0b37f
Compare
| navigate("search/modal") | ||
| }} | ||
| > | ||
| <Flex pointerEvents="none"> |
There was a problem hiding this comment.
just out of curiosity: why do we need pointerEvents="none" here?
There was a problem hiding this comment.
here, this was my solution to avoid building a search input button that looks like the input. So I made the input clickable, but to avoid focusing inside it, I wrapped it with a flex so it doesn't send touch events to its children. And voila, a search input button that's clickable but not focusable
There was a problem hiding this comment.
That's a great idea! 🌟
A short comment in the code to explain what this Flex container does could be good..
| /> | ||
| </Flex> | ||
| </Touchable> | ||
| <GlobalSearchInputModal visible={isVisible} hideModal={() => setIsVisible(false)} /> |
There was a problem hiding this comment.
I don't fully understand why there is a GlobalSearchInputModal and a SearchModalScreen 🤔
There was a problem hiding this comment.
good point - I am actually still trying to figure out the best way to present this modal.
There was a problem hiding this comment.
Overlay, modal or a modally presented screen.
- The overlay would be nice but it would have to be injected on the HomeView as an
absolutepositioned view covering everything - The modal would be perfect, but navigating to an item would require dismissing the modal then, navigating and the transition won't look nice
- having this as a modally presented screen would be idea in this case, however, since it's a modally presented screen, the transition didn't look nice to me
I am checking all options currently again and I am leaning towards the first solution. I know it's what we spoke about during the refinement but I wanted to also explore other approaches
There was a problem hiding this comment.
Thank you very much for explaining 🙏
Intuitively, the first solution sounds like the best solution to me as well.
If using an absolute positioned view covering the home screen does not work, an alternative could be to hide the screen content (e.g., by setting its height to 0).
There was a problem hiding this comment.
I found a way, I did exactly that but without needing to add a store value to show/hide the overlay view on both search and home views. And also without need for prop drilling.
I noticed that we are using react-native-portalas a peer dependency in yarn.lock but we were not importing it anywhere. So I specified that and used it. The API is so straightforward and it worked great
| @@ -114,6 +114,7 @@ | |||
| "@braze/react-native-sdk": "11.0.0", | |||
| "@expo/react-native-action-sheet": "4.0.1", | |||
| "@gorhom/bottom-sheet": "5.0.5", | |||
| "@gorhom/portal": "1.0.14", | |||
There was a problem hiding this comment.
this was already available within yarn.lock. I just specified it here to avoid regressions.
There was a problem hiding this comment.
as you can see, adding it didn't lead to any yarn.lock updates
|
Please don't review yet - I noticed some weird behaviour |
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
d760d65
to
e893df0
Compare
MounirDhahri
force-pushed
the
feat/add-new-search-modal
branch
from
f78e2d4
to
1d8414b
Compare
|
I am going to merge this if no one minds - I am glad to address your comments in a separate PR |
This PR resolves ONYX-1369 ONYX-1368
Description
This PR adds a search button and input to the home view and search tabs.
Screen.Recording.2024-11-12.at.15.35.40.mov
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.