-
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
Migrate search page to chat finder page #40129
Changes from 2 commits
46b6a09
223099a
50bcada
86a01f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,8 @@ if (!appInstanceId) { | |
// import your test here, define its name and config first in e2e/config.js | ||
const tests: Tests = { | ||
[E2EConfig.TEST_NAMES.AppStartTime]: require('./tests/appStartTimeTest.e2e').default, | ||
[E2EConfig.TEST_NAMES.OpenSearchPage]: require('./tests/openSearchPageTest.e2e').default, | ||
// ASK: Should we also update this name? | ||
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. Yes, I think we can update this one since it's just a require`` |
||
[E2EConfig.TEST_NAMES.OpenChatFinderPage]: require('./tests/openChatFinderPageTest.e2e').default, | ||
[E2EConfig.TEST_NAMES.ChatOpening]: require('./tests/chatOpeningTest.e2e').default, | ||
[E2EConfig.TEST_NAMES.ReportTyping]: require('./tests/reportTypingTest.e2e').default, | ||
[E2EConfig.TEST_NAMES.Linking]: require('./tests/linkingTest.e2e').default, | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,7 +9,7 @@ | |||||
|
||||||
const test = () => { | ||||||
// check for login (if already logged in the action will simply resolve) | ||||||
console.debug('[E2E] Logging in for search'); | ||||||
console.debug('[E2E] Logging in for chat finding'); | ||||||
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
|
||||||
|
||||||
E2ELogin().then((neededLogin: boolean): Promise<Response> | undefined => { | ||||||
if (neededLogin) { | ||||||
|
@@ -19,24 +19,24 @@ | |||||
); | ||||||
} | ||||||
|
||||||
console.debug('[E2E] Logged in, getting search metrics and submitting them…'); | ||||||
console.debug('[E2E] Logged in, getting chat finding metrics and submitting them…'); | ||||||
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
|
||||||
|
||||||
Performance.subscribeToMeasurements((entry) => { | ||||||
if (entry.name === CONST.TIMING.SIDEBAR_LOADED) { | ||||||
console.debug(`[E2E] Sidebar loaded, navigating to search route…`); | ||||||
Navigation.navigate(ROUTES.SEARCH); | ||||||
console.debug(`[E2E] Sidebar loaded, navigating to chat finder route…`); | ||||||
Navigation.navigate(ROUTES.CHAT_FINDER); | ||||||
return; | ||||||
} | ||||||
|
||||||
console.debug(`[E2E] Entry: ${JSON.stringify(entry)}`); | ||||||
if (entry.name !== CONST.TIMING.SEARCH_RENDER) { | ||||||
Check failure on line 32 in src/libs/E2E/tests/openChatFinderPageTest.e2e.ts GitHub Actions / typecheck
|
||||||
return; | ||||||
} | ||||||
|
||||||
console.debug(`[E2E] Submitting!`); | ||||||
E2EClient.submitTestResults({ | ||||||
branch: Config.E2E_BRANCH, | ||||||
name: 'Open Search Page TTI', | ||||||
name: 'Open Chat Finder Page TTI', | ||||||
duration: entry.duration, | ||||||
}) | ||||||
.then(() => { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,33 +25,34 @@ import CONST from '@src/CONST'; | |||||||||||
import ONYXKEYS from '@src/ONYXKEYS'; | ||||||||||||
import type SCREENS from '@src/SCREENS'; | ||||||||||||
import type * as OnyxTypes from '@src/types/onyx'; | ||||||||||||
import SearchPageFooter from './SearchPageFooter'; | ||||||||||||
import ChatFinderPageFooter from './ChatFinderPageFooter'; | ||||||||||||
|
||||||||||||
type SearchPageOnyxProps = { | ||||||||||||
type ChatFinderPageOnyxProps = { | ||||||||||||
/** Beta features list */ | ||||||||||||
betas: OnyxEntry<OnyxTypes.Beta[]>; | ||||||||||||
|
||||||||||||
/** Whether or not we are searching for reports on the server */ | ||||||||||||
isSearchingForReports: OnyxEntry<boolean>; | ||||||||||||
}; | ||||||||||||
|
||||||||||||
type SearchPageProps = SearchPageOnyxProps & StackScreenProps<RootStackParamList, typeof SCREENS.SEARCH_ROOT>; | ||||||||||||
type ChatFinderPageProps = ChatFinderPageOnyxProps & StackScreenProps<RootStackParamList, typeof SCREENS.CHAT_FINDER_ROOT>; | ||||||||||||
|
||||||||||||
type SearchPageSectionItem = { | ||||||||||||
type ChatFinderPageSectionItem = { | ||||||||||||
data: OptionData[]; | ||||||||||||
shouldShow: boolean; | ||||||||||||
}; | ||||||||||||
|
||||||||||||
type SearchPageSectionList = SearchPageSectionItem[]; | ||||||||||||
type ChatFinderPageSectionList = ChatFinderPageSectionItem[]; | ||||||||||||
|
||||||||||||
const setPerformanceTimersEnd = () => { | ||||||||||||
Timing.end(CONST.TIMING.SEARCH_RENDER); | ||||||||||||
Performance.markEnd(CONST.TIMING.SEARCH_RENDER); | ||||||||||||
// ASK: Should we also update this name? | ||||||||||||
Timing.end(CONST.TIMING.CHAT_FINDER_RENDER); | ||||||||||||
Performance.markEnd(CONST.TIMING.CHAT_FINDER_RENDER); | ||||||||||||
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. Yea, I think we should update the key, but leave the value of the key as is.
Suggested change
|
||||||||||||
}; | ||||||||||||
|
||||||||||||
const SerachPageFooterInstance = <SearchPageFooter />; | ||||||||||||
const ChatFinderPageFooterInstance = <ChatFinderPageFooter />; | ||||||||||||
|
||||||||||||
function SearchPage({betas, isSearchingForReports, navigation}: SearchPageProps) { | ||||||||||||
function ChatFinderPage({betas, isSearchingForReports, navigation}: ChatFinderPageProps) { | ||||||||||||
const [isScreenTransitionEnd, setIsScreenTransitionEnd] = useState(false); | ||||||||||||
const {translate} = useLocalize(); | ||||||||||||
const {isOffline} = useNetwork(); | ||||||||||||
|
@@ -65,8 +66,9 @@ function SearchPage({betas, isSearchingForReports, navigation}: SearchPageProps) | |||||||||||
const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState(''); | ||||||||||||
|
||||||||||||
useEffect(() => { | ||||||||||||
Timing.start(CONST.TIMING.SEARCH_RENDER); | ||||||||||||
Performance.markStart(CONST.TIMING.SEARCH_RENDER); | ||||||||||||
// ASK: Should we also update this name? | ||||||||||||
Timing.start(CONST.TIMING.CHAT_FINDER_RENDER); | ||||||||||||
Performance.markStart(CONST.TIMING.CHAT_FINDER_RENDER); | ||||||||||||
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
|
||||||||||||
}, []); | ||||||||||||
|
||||||||||||
useEffect(() => { | ||||||||||||
|
@@ -113,8 +115,8 @@ function SearchPage({betas, isSearchingForReports, navigation}: SearchPageProps) | |||||||||||
|
||||||||||||
const {recentReports, personalDetails: localPersonalDetails, userToInvite, headerMessage} = debouncedSearchValue.trim() !== '' ? filteredOptions : searchOptions; | ||||||||||||
|
||||||||||||
const sections = useMemo((): SearchPageSectionList => { | ||||||||||||
const newSections: SearchPageSectionList = []; | ||||||||||||
const sections = useMemo((): ChatFinderPageSectionList => { | ||||||||||||
const newSections: ChatFinderPageSectionList = []; | ||||||||||||
|
||||||||||||
if (recentReports?.length > 0) { | ||||||||||||
newSections.push({ | ||||||||||||
|
@@ -162,12 +164,13 @@ function SearchPage({betas, isSearchingForReports, navigation}: SearchPageProps) | |||||||||||
return ( | ||||||||||||
<ScreenWrapper | ||||||||||||
includeSafeAreaPaddingBottom={false} | ||||||||||||
testID={SearchPage.displayName} | ||||||||||||
testID={ChatFinderPage.displayName} | ||||||||||||
onEntryTransitionEnd={handleScreenTransitionEnd} | ||||||||||||
shouldEnableMaxHeight | ||||||||||||
navigation={navigation} | ||||||||||||
> | ||||||||||||
<HeaderWithBackButton | ||||||||||||
// ASK: What text do we want to use for the title? | ||||||||||||
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. 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. Once we resolve that, I'll record videos for this PR :) 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. @luacmartins can you actually bring this question out into #marketing? This seems like a slightly larger change that we should discuss and align on first in Slack. Thanks! |
||||||||||||
title={translate('common.search')} | ||||||||||||
onBackButtonPress={Navigation.goBack} | ||||||||||||
/> | ||||||||||||
|
@@ -183,21 +186,21 @@ function SearchPage({betas, isSearchingForReports, navigation}: SearchPageProps) | |||||||||||
onLayout={setPerformanceTimersEnd} | ||||||||||||
onSelectRow={selectReport} | ||||||||||||
showLoadingPlaceholder={!areOptionsInitialized || !isScreenTransitionEnd} | ||||||||||||
footerContent={!isDismissed && SerachPageFooterInstance} | ||||||||||||
footerContent={!isDismissed && ChatFinderPageFooterInstance} | ||||||||||||
isLoadingNewOptions={isSearchingForReports ?? undefined} | ||||||||||||
/> | ||||||||||||
</ScreenWrapper> | ||||||||||||
); | ||||||||||||
} | ||||||||||||
|
||||||||||||
SearchPage.displayName = 'SearchPage'; | ||||||||||||
ChatFinderPage.displayName = 'ChatFinderPage'; | ||||||||||||
|
||||||||||||
export default withOnyx<SearchPageProps, SearchPageOnyxProps>({ | ||||||||||||
export default withOnyx<ChatFinderPageProps, ChatFinderPageOnyxProps>({ | ||||||||||||
betas: { | ||||||||||||
key: ONYXKEYS.BETAS, | ||||||||||||
}, | ||||||||||||
isSearchingForReports: { | ||||||||||||
key: ONYXKEYS.IS_SEARCHING_FOR_REPORTS, | ||||||||||||
initWithStoredValues: false, | ||||||||||||
}, | ||||||||||||
})(SearchPage); | ||||||||||||
})(ChatFinderPage); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,8 @@ const OUTPUT_DIR = process.env.WORKING_DIRECTORY || './tests/e2e/results'; | |
// add your test name here … | ||
const TEST_NAMES = { | ||
AppStartTime: 'App start time', | ||
OpenSearchPage: 'Open search page TTI', | ||
// ASK: Should we also update this name? | ||
OpenChatFinderPage: 'Open chat finder page TTI', | ||
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. Yea, we should. I don't think this value is being saved anywhere (at least I couldn't find that in code) |
||
ReportTyping: 'Report typing', | ||
ChatOpening: 'Chat opening', | ||
Linking: 'Linking', | ||
|
@@ -71,8 +72,8 @@ export default { | |
|
||
// ... any additional config you might need | ||
}, | ||
[TEST_NAMES.OpenSearchPage]: { | ||
name: TEST_NAMES.OpenSearchPage, | ||
[TEST_NAMES.OpenChatFinderPage]: { | ||
name: TEST_NAMES.OpenChatFinderPage, | ||
}, | ||
// TODO: Fix text and enable again | ||
// [TEST_NAMES.ReportTyping]: { | ||
|
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 it seems like we save this metric to Firebase and Grafana. So Maybe we should just rename the key, but leave the value as is.