Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Android): Change elements visibility on search bar open #1903

Merged
merged 1 commit into from
Sep 30, 2023
Merged

fix(Android): Change elements visibility on search bar open #1903

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions android/src/main/java/com/swmansion/rnscreens/SearchBarView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,19 @@ class SearchBarView(reactContext: ReactContext?) : ReactViewGroup(reactContext)

private var mAreListenersSet: Boolean = false

private val screenStackFragment: ScreenStackFragment?
private val headerConfig: ScreenStackHeaderConfig?
get() {
val currentParent = parent
if (currentParent is ScreenStackHeaderSubview) {
return currentParent.config?.screenFragment
return currentParent.config
}

return null
}

private val screenStackFragment: ScreenStackFragment?
get() = headerConfig?.screenFragment

fun onUpdate() {
setSearchViewProps()
}
Expand Down Expand Up @@ -110,10 +114,12 @@ class SearchBarView(reactContext: ReactContext?) : ReactViewGroup(reactContext)

private fun handleClose() {
sendEvent(SearchBarCloseEvent(surfaceId, id))
setToolbarElementsVisibility(VISIBLE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility that this won't be fired, for example is we navigated when the search bar was open?

Copy link
Member Author

@tboba tboba Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WoLewicki I thought about that already and I'm unable to reproduce the scenario where the user comes back from the search bar and in the same action comes to the previous screen. Is it even possible? 🤔

Screen.Recording.2023-09-29.at.13.21.50.mov

To test this I've changed the logic of handleClose to change the visibility of elements to INVISIBLE - that's why the element on the current screen hides.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I figured out how to do that (I'm calling navigation.navigate('Main') on onClose) and it's doing fine!

Screen.Recording.2023-09-29.at.13.53.02.mov

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better example would be to navigate forward and see what happens when you go back.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean navigation forward without opening the search bar? If so, then nothing would happen - state of the header subviews is only being changed on open and close searchBar events. If you mean that I should navigate forward, open the search bar and try to close it with trying to go back with back button, then (as seen above):

  1. The keyboard will close,
  2. The search bar will close,
  3. User will return to previous screen.

So it shouldn't hide, as it's not possible to return to the previous screen without closing the search bar, I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WoLewicki So I've checked the scenario that you've suggested and looks like everything still works - I can't manage to have search bar closed when I'm going to the next screen and coming to the previous one.

8mb.video-bqT-PgqBQjkb.mp4

I'm still thinking if changing the visibility of the element could still break at some point. Do you think we should correct the change just to make sure that we will avoid side-effects?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be some weird edge case and we can leave it as it is for now, and if the issues arise in the future, we will fix it then.

}

private fun handleOpen() {
sendEvent(SearchBarOpenEvent(surfaceId, id))
setToolbarElementsVisibility(GONE)
}

private fun handleTextSubmit(newText: String?) {
Expand Down Expand Up @@ -144,6 +150,14 @@ class SearchBarView(reactContext: ReactContext?) : ReactViewGroup(reactContext)
text?.let { screenStackFragment?.searchView?.setText(it) }
}

private fun setToolbarElementsVisibility(visibility: Int) {
for (i in 0..(headerConfig?.configSubviewsCount?.minus(1) ?: 0)) {
tboba marked this conversation as resolved.
Show resolved Hide resolved
val subview = headerConfig?.getConfigSubview(i)
if (subview?.type != ScreenStackHeaderSubview.Type.SEARCH_BAR)
subview?.visibility = visibility
}
}

private val surfaceId = UIManagerHelper.getSurfaceId(this)

enum class SearchBarAutoCapitalize {
Expand Down
Loading