fix: No results found issue and refactor & merge NewGroupPage into NewChatPage

[parasharrajat]

Details

#4993 (comment)

Fixed Issues

$ #4993

Tests | QA Steps

No result found case

1. Go to staging.new.expensify.com
2. Login with any account.
3. Tap + green fab and select "New chat" option
4. On the search box, search any user you have access.
5. You should not see No result found if a user is found.

New Chat Page functionality

1. Test the New Chat Page is functioning correctly as it should.

New Group Page functionality

1. Test the New Group Page is functioning correctly as it should.

Split Bill participants selection functionality

1. Test the Split Bill participants selection is functioning correctly as it should.

Request Money participants selection functionality

1. Test the Request Money participants selection is functioning correctly as it should.

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web | Desktop

Mobile Web

iOS

Android

[kadiealexander]

Hi @parasharrajat, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

[NikkiWines]

@parasharrajat, just letting you know that this is on my radar, I'm just holding on my review of this until the N6-hold no longer applies

[parasharrajat]

Thanks, @NikkiWines. I will probably ask @roryabraham to confirm the changes here as he requested refactoring.

[NikkiWines]

Adding [HOLD] to the title of this per https://expensify.slack.com/archives/C01GTK53T8Q/p1633978544125600

[NikkiWines]

N6-Hold is now over. @parasharrajat please merge master and re-test this PR, thank you! 🙇

[parasharrajat]

@NikkiWines I have updated the code and tested it.

cc: @roryabraham I have refactored the NewGroupPage in the NewChatPage.

[NikkiWines]

This looks good but I would recommend adding some QA steps for testing creating a new chat and a new group chat just to make sure everything is being thoroughly tested.

[parasharrajat]

What a mess another conflict... For QA steps: I have added that those pages should be retested completely. There are no special steps. just the complete functionality for mentioned pages.

[parasharrajat]

Check passed Ready to roll. @NikkiWines

[NikkiWines]

looks good 👍 all yours @roryabraham

[roryabraham]

Looks great @parasharrajat! Just had a few minor comments and also there's conflicts again. Sorry for the delayed review here.

[parasharrajat]

@roryabraham @NikkiWines Changes done.

[roryabraham]

Looks like we've got a lint error.

[parasharrajat]

Sorry Just fixed that.

[parasharrajat]

@roryabraham Ready to merge.

[NikkiWines]

[parasharrajat]

@roryabraham Gentle Bump.

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[OSBotify]

🚀 Deployed to staging by @roryabraham in version: 1.1.8-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[marcaaron]

Hey this is a nice cleanup!

Just wanted to drop some feedback that maybe we could avoid creating components that switch from one "type" to another depending on a single prop. This practice feels wrong to me and I wanted to suggest we break that up into a few more props...

App/src/pages/NewChatPage.js

Line 79 in ef9b8bb

this.props.isGroupChat ? this.excludedGroupEmails : [],

This could be changed to an excludedLogins prop that defaults to an empty array

App/src/pages/NewChatPage.js

Lines 98 to 109 in ef9b8bb

	if (this.props.isGroupChat) {
	sections.push({
	title: undefined,
	data: this.state.selectedOptions,
	shouldShow: true,
	indexOffset: 0,
	});
	if (maxParticipantsReached) {
	return sections;
	}
	}

This could be canSelectMultipleOptions

App/src/pages/NewChatPage.js

Lines 204 to 206 in ef9b8bb

	if (this.props.isGroupChat) {
	return this.toggleOption(option);
	}

Also canSelectMultipleOptions

App/src/pages/NewChatPage.js

Lines 225 to 227 in ef9b8bb

	title={this.props.isGroupChat
	? this.props.translate('sidebarScreen.newGroup')
	: this.props.translate('sidebarScreen.newChat')}

title

App/src/pages/NewChatPage.js

Lines 265 to 275 in ef9b8bb

	{this.props.isGroupChat && lodashGet(this.state, 'selectedOptions', []).length > 0 && (
	<FixedFooter>
	<Button
	success
	onPress={this.createGroup}
	style={[styles.w100]}
	text={this.props.translate('newChatPage.createGroup')}
	/>
	</FixedFooter>
	)}
	</>

canSelectMultipleOptions

This way the page has no knowledge of whether it is a "group" page just which behavior it needs to do. I hope this makes sense :)

[parasharrajat]

Yup @marcaaron these are great suggestions. At that time I just thought of merging two pages into one. But yeah this is a good mindset to have for future.

[OSBotify]

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅