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

Jump to bottom

Add index to key for contact search recommendations #20186

Merged
merged 1 commit into from Oct 7, 2019
Merged

Add index to key for contact search recommendations #20186

Show file tree
Hide file tree
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions shared/team-building/__test__/team-building.test.tsx
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ import {SectionDivider} from '../../common-adapters/index'
describe('team building list', () => {
it('calculates list offsets properly', () => {
const testSearchResult: SearchResult = {
contact: false,
displayLabel: '',
followingState: 'NotFollowing',
inTeam: false,
Expand Down
15 changes: 15 additions & 0 deletions shared/team-building/index.stories.tsx
View file
Open in desktop
Expand Up @@ -150,6 +150,7 @@ const load = () => {
search={Sb.action('search')}
searchResults={[
{
contact: false,
displayLabel: 'Chris Coyne',
followingState: 'Following' as const,
inTeam: true,
Expand All @@ -167,6 +168,7 @@ const load = () => {
username: 'chris',
},
{
contact: false,
displayLabel: 'Chris Mikacle',
followingState: 'NotFollowing' as const,
inTeam: false,
Expand All @@ -183,6 +185,7 @@ const load = () => {
username: 'chrismikacle',
},
{
contact: false,
displayLabel: 'Chris Nojima',
followingState: 'Following' as const,
inTeam: false,
Expand Down Expand Up @@ -244,6 +247,7 @@ const load = () => {
search={Sb.action('search')}
searchResults={[
{
contact: false,
displayLabel: 'Chris Coyne',
followingState: 'Following' as const,
inTeam: true,
Expand All @@ -261,6 +265,7 @@ const load = () => {
username: 'chris',
},
{
contact: false,
displayLabel: 'Chris Mikacle',
followingState: 'NotFollowing' as const,
inTeam: false,
Expand All @@ -277,6 +282,7 @@ const load = () => {
username: 'chrismikacle',
},
{
contact: false,
displayLabel: 'Chris Nojima',
followingState: 'Following' as const,
inTeam: false,
Expand Down Expand Up @@ -385,6 +391,7 @@ const load = () => {
search={Sb.action('search')}
searchResults={[
{
contact: false,
displayLabel: 'Chris Coyne',
followingState: 'Following' as const,
inTeam: true,
Expand All @@ -402,6 +409,7 @@ const load = () => {
username: 'chris',
},
{
contact: false,
displayLabel: 'Chris Mikacle',
followingState: 'NotFollowing' as const,
inTeam: false,
Expand All @@ -418,6 +426,7 @@ const load = () => {
username: 'chrismikacle',
},
{
contact: false,
displayLabel: 'Chris Nojima',
followingState: 'Following' as const,
inTeam: false,
Expand Down Expand Up @@ -460,6 +469,7 @@ const load = () => {
search={Sb.action('search')}
searchResults={[
{
contact: false,
displayLabel: 'Chris Coyne',
followingState: 'Following' as const,
inTeam: true,
Expand All @@ -477,6 +487,7 @@ const load = () => {
username: 'chris',
},
{
contact: false,
displayLabel: 'Chris Mikacle',
followingState: 'NotFollowing' as const,
inTeam: false,
Expand All @@ -493,6 +504,7 @@ const load = () => {
username: 'chrismikacle',
},
{
contact: false,
displayLabel: 'Chris Nojima',
followingState: 'Following' as const,
inTeam: false,
Expand Down Expand Up @@ -535,6 +547,7 @@ const load = () => {
search={Sb.action('search')}
searchResults={[
{
contact: false,
displayLabel: 'Chris Coyne',
followingState: 'Following' as const,
inTeam: true,
Expand All @@ -552,6 +565,7 @@ const load = () => {
username: 'chris',
},
{
contact: false,
displayLabel: 'Chris Mikacle',
followingState: 'NotFollowing' as const,
inTeam: false,
Expand All @@ -568,6 +582,7 @@ const load = () => {
username: 'chrismikacle',
},
{
contact: false,
displayLabel: 'Chris Nojima',
followingState: 'Following' as const,
inTeam: false,
Expand Down
7 changes: 6 additions & 1 deletion shared/team-building/index.tsx
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@ import * as Constants from '../constants/team-building'
export const numSectionLabel = '0-9'

export type SearchResult = {
contact: boolean
userId: string
username: string
prettyName: string
Expand Down Expand Up @@ -413,7 +414,11 @@ class TeamBuilding extends React.PureComponent<Props, {}> {
stickySectionHeadersEnabled={false}
selectedIndex={Styles.isMobile ? undefined : this.props.highlightedIndex || 0}
sections={this.props.recommendations}
keyExtractor={(item: SearchResult | ImportContactsEntry) => {
keyExtractor={(item: SearchResult | ImportContactsEntry, index: number) => {
if (!isImportContactsEntry(item) && item.contact) {
// Ids for contacts are not guaranteed to be unique
return item.userId + index
Copy link
Contributor

Choose a reason for hiding this comment

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

why not key all rows like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

(except for "Import contacts" row)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's at least a chance that a recommendation row will be reused as a search result, and don't want to disqualify that from happening

}
return isImportContactsEntry(item) ? 'Import Contacts' : item.userId
}}
getItemLayout={this._getRecLayout}
Expand Down