Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.view.ViewCompat
import androidx.core.view.WindowInsetsAnimationCompat
import androidx.core.view.WindowInsetsCompat
import androidx.core.view.children
import com.facebook.react.uimanager.PixelUtil
import com.facebook.react.uimanager.UIManagerHelper
import com.google.android.material.appbar.AppBarLayout
Expand Down Expand Up @@ -412,32 +413,43 @@ class ScreenStackFragment :
return super.onCreateOptionsMenu(menu, inflater)
}

private fun shouldShowSearchBar(): Boolean {
private fun locateSearchBarSubviewInConfig(): ScreenStackHeaderSubview? {
val config = screen.headerConfig
val numberOfSubViews = config?.configSubviewsCount ?: 0
if (config != null && numberOfSubViews > 0) {
for (i in 0 until numberOfSubViews) {
val subView = config.getConfigSubview(i)
if (subView.type == ScreenStackHeaderSubview.Type.SEARCH_BAR) {
return true
return subView
}
}
}
return false
return null
}

private fun extractShowAsActionFromSearchBar(searchSubview: ScreenStackHeaderSubview): Int {
for (child in searchSubview.children) {
if (child is SearchBarView) {
return child.showAsAction
}
}

return MenuItem.SHOW_AS_ACTION_ALWAYS
}

private fun updateToolbarMenu(menu: Menu) {
menu.clear()
if (shouldShowSearchBar()) {
locateSearchBarSubviewInConfig()?.let {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

shouldShowSearchBar() was effectively true when config for type==SEARCH_BAR existed, so this refactor should not change the behaviour

Copy link
Member

Choose a reason for hiding this comment

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

This is ok, thanks for the note.

val currentContext = context
if (searchView == null && currentContext != null) {
val newSearchView = CustomSearchView(currentContext, this)
searchView = newSearchView
onSearchViewCreate?.invoke(newSearchView)
}
menu.add("").apply {
setShowAsAction(MenuItem.SHOW_AS_ACTION_ALWAYS)
menu.add("Search").apply {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to change that?

Copy link
Member

Choose a reason for hiding this comment

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

Where is this title used?

actionView = searchView
setIcon(R.drawable.ic_action_search)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to set this icon manually?

setShowAsAction(extractShowAsActionFromSearchBar(it))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ class SearchBarManager :
view.shouldOverrideBackButton = disableBackButtonOverride != true
}

@ReactProp(name = "searchShowAsAction")
override fun setShowAsAction(
view: SearchBarView,
showAsAction: String?,
) {
view.setShowAsAction(showAsAction)
}

@ReactProp(name = "inputType")
override fun setInputType(
view: SearchBarView,
Expand Down
12 changes: 12 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/SearchBarView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.swmansion.rnscreens

import android.annotation.SuppressLint
import android.text.InputType
import android.view.MenuItem
import androidx.appcompat.widget.SearchView
import com.facebook.react.bridge.ReactContext
import com.facebook.react.uimanager.UIManagerHelper
Expand Down Expand Up @@ -30,6 +31,17 @@ class SearchBarView(
var autoFocus: Boolean = false
var shouldShowHintSearchIcon: Boolean = true

var showAsAction: Int = MenuItem.SHOW_AS_ACTION_ALWAYS
private set

fun setShowAsAction(actionView: String?) {
showAsAction = when (actionView) {
"collapse" -> MenuItem.SHOW_AS_ACTION_ALWAYS or MenuItem.SHOW_AS_ACTION_COLLAPSE_ACTION_VIEW
"always" -> MenuItem.SHOW_AS_ACTION_ALWAYS
else -> MenuItem.SHOW_AS_ACTION_ALWAYS
}
}

private var searchViewFormatter: SearchViewFormatter? = null

private var areListenersSet: Boolean = false
Expand Down
11 changes: 11 additions & 0 deletions android/src/main/res/base/drawable-anydpi/ic_action_search.xml
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this additional resource?

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24"
android:tint="#333333"
android:alpha="0.6">
<path
android:fillColor="@android:color/white"
android:pathData="M15.5,14h-0.79l-0.28,-0.27C15.41,12.59 16,11.11 16,9.5 16,5.91 13.09,3 9.5,3S3,5.91 3,9.5 5.91,16 9.5,16c1.61,0 3.09,-0.59 4.23,-1.57l0.27,0.28v0.79l5,4.99L20.49,19l-4.99,-5zM9.5,14C7.01,14 5,11.99 5,9.5S7.01,5 9.5,5 14,7.01 14,9.5 11.99,14 9.5,14z"/>
</vector>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ public void setProperty(T view, String propName, @Nullable Object value) {
case "disableBackButtonOverride":
mViewManager.setDisableBackButtonOverride(view, value == null ? false : (boolean) value);
break;
case "showAsAction":
mViewManager.setShowAsAction(view, (String) value);
break;
case "inputType":
mViewManager.setInputType(view, value == null ? null : (String) value);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public interface RNSSearchBarManagerInterface<T extends View> {
void setTintColor(T view, @Nullable Integer value);
void setTextColor(T view, @Nullable Integer value);
void setDisableBackButtonOverride(T view, boolean value);
void setShowAsAction(T view, @Nullable String value);
void setInputType(T view, @Nullable String value);
void setHintTextColor(T view, @Nullable Integer value);
void setHeaderIconColor(T view, @Nullable Integer value);
Expand Down
67 changes: 67 additions & 0 deletions apps/src/tests/Test2744.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from 'react';
import { NavigationContainer, ParamListBase } from '@react-navigation/native';
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
import { Button, View, ScrollView } from 'react-native';

type StackRouteParamList = {
Home: undefined;
SearchAlways: undefined;
SearchCollapse: undefined;
};

type NavigationProp<ParamList extends ParamListBase> = {
navigation: NativeStackNavigationProp<ParamList>;
};

type StackNavigationProp = NavigationProp<StackRouteParamList>;

const SearchAlways = ({navigation}: StackNavigationProp) => {
return <ScrollView contentInsetAdjustmentBehavior='automatic'>
Copy link
Member

Choose a reason for hiding this comment

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

Let's use braces () - this will make code more maintainable, if some additional logic will be needed, also it'll stick to the pattern we use everywhere else.

Another thing - in general I prefer (and try to enforce) usage of regular functions over arrow functions, due to symbol hoisting. Therefore let's rewrite this a bit.

<Button onPress={() => navigation.pop()} title='Go back'/>
</ScrollView>
}

const SearchCollapse = ({navigation}: StackNavigationProp) => {
return <ScrollView contentInsetAdjustmentBehavior='automatic'>
<Button onPress={() => navigation.pop()} title='Go back'/>
</ScrollView>
}

const Home = ({navigation}: StackNavigationProp) =>
<View>
<Button onPress={() => navigation.push("SearchAlways")} title='showAsAction=always'/>
<Button onPress={() => navigation.push("SearchCollapse")} title='showAsAction=collapse'/>
</View>

const Stack = createNativeStackNavigator<StackRouteParamList>();

export default function App() {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen
name="Home"
component={Home}
/>
<Stack.Screen
name="SearchAlways"
component={SearchAlways}
options={{
title: "showAsAction=default",
headerSearchBarOptions: {}
}}
/>
<Stack.Screen
name="SearchCollapse"
component={SearchCollapse}
options={{
title: "showAsAction=collapse",
headerSearchBarOptions: {
showAsAction: "collapse"
}
}}
/>
</Stack.Navigator>
</NavigationContainer>
);
}
1 change: 1 addition & 0 deletions apps/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ export { default as Test2668 } from './Test2668';
export { default as Test2675 } from './Test2675';
export { default as Test2714 } from './Test2714';
export { default as Test2717 } from './Test2717';
export { default as Test2744 } from './Test2744';
export { default as Test2767 } from './Test2767';
export { default as Test2789 } from './Test2789';
export { default as Test2809 } from './Test2809';
Expand Down
3 changes: 3 additions & 0 deletions src/fabric/SearchBarNativeComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ type SearchBarPlacement = 'automatic' | 'inline' | 'stacked';

type AutoCapitalizeType = 'none' | 'words' | 'sentences' | 'characters';

type SearchShowAsAction = 'default' | 'always' | 'collapse';
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need default case? Couldn't we just use always if the user does not set the value? Does it give us any advantage over direct approach?


export interface NativeProps extends ViewProps {
onSearchFocus?: DirectEventHandler<SearchBarEvent> | null;
onSearchBlur?: DirectEventHandler<SearchBarEvent> | null;
Expand All @@ -43,6 +45,7 @@ export interface NativeProps extends ViewProps {

// Android only
disableBackButtonOverride?: boolean;
showAsAction?: WithDefault<SearchShowAsAction, 'default'>
// TODO: consider creating enum here
inputType?: string;
onClose?: DirectEventHandler<SearchBarEvent> | null;
Expand Down
11 changes: 11 additions & 0 deletions src/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export type HeaderSubviewTypes =
| 'left'
| 'center'
| 'searchBar';
export type SearchShowAsAction = 'default' | 'always' | 'collapse';

export type HeaderHeightChangeEventType = {
headerHeight: number;
Expand Down Expand Up @@ -733,6 +734,16 @@ export interface SearchBarProps {
* @platform ios
*/
tintColor?: ColorValue;
/**
* @platform android
Copy link
Member

Choose a reason for hiding this comment

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

This annotation should be placed in the end of the comment block. No technical reason, just to keep the convention.

*
* Controls the behavior of SearchView on the Toolbar/ActionBar. When set to `collapse`,
* clicking 'Up' for the first time takes the focus from the SearchView, and current screen
* is popped on the second click. When set to `default` or `always`, current screen is dismissed immediately.
* This is equivalent to setting android's `showAsAction` to `SHOW_AS_ACTION_ALWAYS`
* or `SHOW_AS_ACTION_ALWAYS | SHOW_AS_ACTION_COLLAPSE_ACTION_VIEW`.
*/
showAsAction?: SearchShowAsAction;
/**
* The text to be used instead of default `Cancel` button text
*
Expand Down
Loading