[MM-60405] Crossteam search #8411

JulienTant · 2024-12-12T19:35:44Z

Summary

This PR changes to design to have the new team selector in the search and result views, and adds a new "All teams" option only if the correct feature flag is set on the server.

Tickets Link

https://mattermost.atlassian.net/browse/MM-60405
https://mattermost.atlassian.net/browse/MM-60410
https://mattermost.atlassian.net/browse/MM-60411

Blocked by

mattermost/mattermost#29389

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.
Have run E2E tests by adding label E2E iOS tests for PR.

Device Information

This PR was tested on: iOS Emulator, Android emulator, iPad emulator

Screenshots

New team selector in the search page

The team bottom sheet contains "All Teams" (FF is on)

New team selector in the result page

Release Note

NONE

enahum · 2024-12-17T04:19:56Z

Before I even begin reviewing this, did you check Android and iPad?

JulienTant · 2024-12-17T17:16:49Z

Before I even begin reviewing this, did you check Android and iPad?

Yep, I tried them both in their respective emulators.

enahum · 2024-12-19T04:38:13Z

Before I even begin reviewing this, did you check Android and iPad?

Yep, I tried them both in their respective emulators.

that is nice, can you share screenshots or video captures for UX reference?

enahum · 2024-12-19T04:39:34Z

app/components/team_list/index.test.tsx

what % of code is being covered with this tests? if 80% or more then OK, if not, let's try and bring it to 80%+ please.

We should be good, jest --coverage app/components/team_list/index.test.tsx --collectCoverageFrom=app/components/team_list/index.tsx gave me

Statements : 100% ( 16/16 ) Branches : 91.66% ( 11/12 ) Functions : 100% ( 5/5 ) Lines : 100% ( 14/14 )

I had to change the jest.config.js which is set to not report coverage on our components or screens.

app/components/team_list/index.test.tsx

enahum · 2024-12-19T05:13:39Z

app/components/team_list/team_list_item/team_list_item.tsx

@@ -20,6 +20,7 @@ type Props = {
    iconBackgroundColor?: string;
    selectedTeamId?: string;
    onPress: (teamId: string) => void;
+    hideIcon?: boolean;


Why are we hiding the icon? I could not find any reference to this in the tickets nor a figma attached

The new design calls for no icons in the team selector for search, from Figma:

enahum · 2024-12-19T05:14:21Z

app/components/team_list/team_list_item/team_list_item.tsx

+                        testID={`${teamListItemTestId}.team_icon`}
+                    />
+                </View>
+            }
            <Text
                style={[styles.text, Boolean(textColor) && {color: textColor}]}
                numberOfLines={1}


for long names, should we add an ellipsis ?

It's already handled!

enahum · 2024-12-19T05:15:20Z

app/screens/home/search/index.tsx

    };
 });

+export const ALL_TEAMS_ID = '';


perhaps this should be moved to a constants file.

I didn't notice the constants folder! Thanks, I moved it there!

enahum · 2024-12-19T05:32:57Z

app/screens/home/search/results/header.tsx

            marginBottom: 12,
            paddingHorizontal: 12,
            flexDirection: 'row',
+            justifyContent: 'space-between',


please show how is this modifying the UI and the impact, I know we spend a long time making this right, so I want to avoid changes here if they don't match the UI unless the UI changes for this are expected.

I made a screenshot on iOS and Android, and here's the link to Figma: https://www.figma.com/design/afCozEMCPeyfomstQfyp61/Search-Across-Teams?node-id=4274-35733&p=f&t=tlOZueT4DGgmwuhq-0

iPhone:

Android:

enahum · 2024-12-19T05:35:16Z

app/screens/home/search/team_picker.tsx

-                        </View>
+                    <View style={{flex: 1}}>
+                        <Text
+                            id={selectedTeam.id}


shouldn't we add an ellipsis here? also why are we removing the icon?

The ellipsis is handled:

And the icon removal is part of the new design

enahum · 2024-12-19T05:35:29Z

app/screens/home/search/team_picker.tsx

-                                smallText={true}
-                            />
-                        </View>
+                    <View style={{flex: 1}}>


move to a stylesheet

enahum · 2024-12-19T05:35:56Z

app/screens/home/search/team_picker.tsx

                        <CompassIcon
                            color={changeOpacity(theme.centerChannelColor, 0.56)}
-                            style={styles.compass}


why remove?

It was not necessary anymore

enahum · 2024-12-19T05:36:17Z

ios/Gekidou/Package.resolved

revert the changes to this file

Co-authored-by: Elias Nahum <nahumhbl@gmail.com>

…to MM-60405

JulienTant added 5 commits December 10, 2024 16:30

Add new team picker for search

88f35d6

try fix result header

76a710c

fix style

1961251

add test for team picker

79c72ad

add some tests

8fad4b4

mm-cloud-bot added the release-note label Dec 12, 2024

add tests on team list and team list item

199f9a9

JulienTant added the Work In Progress Not yet ready for review label Dec 12, 2024

JulienTant marked this pull request as ready for review December 12, 2024 20:03

amyblais added this to the v2.25.0 milestone Dec 13, 2024

JulienTant added 2: Dev Review Requires review by a core commiter and removed Work In Progress Not yet ready for review labels Dec 16, 2024

JulienTant requested review from rahimrahman and enahum December 16, 2024 16:12

JulienTant added 2 commits December 16, 2024 16:06

hide All Teams search behind FF

7507ceb

use style variable for separator

b59c05a

JulienTant added the E2E iOS tests for PR Run iOS E2E Detox tests label Dec 16, 2024

github-actions bot removed the E2E iOS tests for PR Run iOS E2E Detox tests label Dec 17, 2024

JulienTant added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Dec 17, 2024

ALL TEAMS does not have a search history

9a9e341

enahum requested changes Dec 19, 2024

View reviewed changes

JulienTant and others added 6 commits December 19, 2024 09:59

Update app/components/team_list/index.test.tsx

fa204a4

Co-authored-by: Elias Nahum <nahumhbl@gmail.com>

Merge branch 'MM-60405' of github.com:mattermost/mattermost-mobile in…

de166ad

…to MM-60405

move ALL_TEAMS_ID to a constant file

e650f51

memoize the construction of team list to prevent useless allocation

77f5ab0

combine pushes

ba34d8e

move style to stylesheet

7ed6dd8

revert changes to Package.resolved

220a9d8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-60405] Crossteam search #8411

[MM-60405] Crossteam search #8411

JulienTant commented Dec 12, 2024 •

edited

Loading

enahum commented Dec 17, 2024

JulienTant commented Dec 17, 2024

enahum commented Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024 •

edited

Loading

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

JulienTant Dec 19, 2024

enahum Dec 19, 2024

[MM-60405] Crossteam search #8411

Are you sure you want to change the base?

[MM-60405] Crossteam search #8411

Conversation

JulienTant commented Dec 12, 2024 • edited Loading

Summary

Tickets Link

Blocked by

Checklist

Device Information

Screenshots

Release Note

enahum commented Dec 17, 2024

JulienTant commented Dec 17, 2024

enahum commented Dec 19, 2024

Choose a reason for hiding this comment

JulienTant Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienTant commented Dec 12, 2024 •

edited

Loading

JulienTant Dec 19, 2024 •

edited

Loading