-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
[Search v1] Create Search widget #40903
Merged
luacmartins
merged 4 commits into
Expensify:main
from
software-mansion-labs:kicu/search-v1/search-widget
Apr 26, 2024
+221
−144
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 18 additions & 0 deletions
18
src/components/SelectionList/TemporaryTransactionListItem.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import React from 'react'; | ||
import {View} from 'react-native'; | ||
import Text from '@components/Text'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
import type {SearchTransaction} from '@src/types/onyx/SearchResults'; | ||
|
||
// NOTE: This is a completely temporary mock item so that something can be displayed in SearchWidget | ||
// This should be removed and implement properly in: https://github.com/Expensify/App/issues/39877 | ||
function TransactionListItem({item}: {item: SearchTransaction}) { | ||
const styles = useThemeStyles(); | ||
return ( | ||
<View style={[styles.pt8]}> | ||
<Text>Item: {item.transactionID}</Text> | ||
</View> | ||
); | ||
} | ||
|
||
export default TransactionListItem; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
type SearchParams = { | ||
query: string; | ||
hash: number; | ||
}; | ||
|
||
export default SearchParams; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import TransactionListItem from '@components/SelectionList/TemporaryTransactionListItem'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
import type * as OnyxTypes from '@src/types/onyx'; | ||
import type {SearchTransaction} from '@src/types/onyx/SearchResults'; | ||
import * as UserUtils from './UserUtils'; | ||
|
||
function getTransactionsSections(data: OnyxTypes.SearchResults['data']): SearchTransaction[] { | ||
return Object.entries(data) | ||
.filter(([key]) => key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION)) | ||
.map(([, value]) => value); | ||
} | ||
|
||
const searchTypeToItemMap = { | ||
transaction: { | ||
listItem: TransactionListItem, | ||
getSections: getTransactionsSections, | ||
}, | ||
}; | ||
|
||
/** | ||
* TODO: in future make this function generic and return specific item component based on type | ||
* For now only 1 search item type exists in the app so this function is simplified | ||
*/ | ||
luacmartins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
function getListItem(): typeof TransactionListItem { | ||
return searchTypeToItemMap.transaction.listItem; | ||
} | ||
|
||
luacmartins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
function getSections(data: OnyxTypes.SearchResults['data']): SearchTransaction[] { | ||
return searchTypeToItemMap.transaction.getSections(data); | ||
} | ||
|
||
function getQueryHash(query: string): number { | ||
return UserUtils.hashText(query, 2 ** 32); | ||
} | ||
|
||
export {getQueryHash, getListItem, getSections}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import Onyx from 'react-native-onyx'; | ||
import * as API from '@libs/API'; | ||
import {READ_COMMANDS} from '@libs/API/types'; | ||
import * as SearchUtils from '@libs/SearchUtils'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
||
let isNetworkOffline = false; | ||
Onyx.connect({ | ||
key: ONYXKEYS.NETWORK, | ||
callback: (value) => { | ||
isNetworkOffline = value?.isOffline ?? false; | ||
}, | ||
}); | ||
|
||
function search(query: string) { | ||
if (isNetworkOffline) { | ||
return; | ||
} | ||
|
||
const hash = SearchUtils.getQueryHash(query); | ||
API.read(READ_COMMANDS.SEARCH, {query, hash}); | ||
} | ||
|
||
export { | ||
// eslint-disable-next-line import/prefer-default-export | ||
search, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,7 +1,6 @@ | ||||||
import React from 'react'; | ||||||
import {View} from 'react-native'; | ||||||
import MenuItem from '@components/MenuItem'; | ||||||
import useActiveRoute from '@hooks/useActiveRoute'; | ||||||
import useLocalize from '@hooks/useLocalize'; | ||||||
import useSingleExecution from '@hooks/useSingleExecution'; | ||||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||
|
@@ -14,17 +13,20 @@ import ROUTES from '@src/ROUTES'; | |||||
import type IconAsset from '@src/types/utils/IconAsset'; | ||||||
import SearchFiltersNarrow from './SearchFiltersNarrow'; | ||||||
|
||||||
type SearchFiltersProps = { | ||||||
query: string; | ||||||
}; | ||||||
|
||||||
type SearchMenuFilterItem = { | ||||||
title: string; | ||||||
icon: IconAsset; | ||||||
route: Route; | ||||||
}; | ||||||
|
||||||
function SearchFilters() { | ||||||
function SearchFilters({query}: SearchFiltersProps) { | ||||||
const styles = useThemeStyles(); | ||||||
const {singleExecution} = useSingleExecution(); | ||||||
const activeRoute = useActiveRoute(); | ||||||
const {isSmallScreenWidth} = useWindowDimensions(); | ||||||
const {singleExecution} = useSingleExecution(); | ||||||
const {translate} = useLocalize(); | ||||||
|
||||||
const filterItems: SearchMenuFilterItem[] = [ | ||||||
|
@@ -35,23 +37,19 @@ function SearchFilters() { | |||||
}, | ||||||
]; | ||||||
|
||||||
const currentQuery = activeRoute?.params && 'query' in activeRoute.params ? activeRoute?.params?.query : ''; | ||||||
|
||||||
if (isSmallScreenWidth) { | ||||||
const activeItemLabel = String(currentQuery); | ||||||
|
||||||
return ( | ||||||
<SearchFiltersNarrow | ||||||
filterItems={filterItems} | ||||||
activeItemLabel={activeItemLabel} | ||||||
activeItemLabel={String(query)} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not block on this. We can address this in a follow up @Kicu @WojtekBoman |
||||||
/> | ||||||
); | ||||||
} | ||||||
|
||||||
return ( | ||||||
<View style={[styles.pb4, styles.mh3, styles.mt3]}> | ||||||
{filterItems.map((item) => { | ||||||
const isActive = item.title.toLowerCase() === currentQuery; | ||||||
const isActive = item.title.toLowerCase() === query; | ||||||
const onPress = singleExecution(() => Navigation.navigate(item.route)); | ||||||
|
||||||
return ( | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I found it to be a bit weird that refreshing the page doesn't trigger a call to search, but that's something we could iron out later in a follow up if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the
search()
gets called, but its the implementation ofisNetworkOffline
thats breaking it.I did some logging and in the beginning the NETWORK sets
isOffline
to true for a split second, and that is when the useEffect triggers. Then a bit laterisOffline
is correctly set tofalse
but by then effect has already run and no search call was made.Not sure how to fix this, need some guidance from you. Is that the intended behavior of
ONYXKEYS.NETWORK
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's a blocker for now. Let's move on and address this as a follow up.