Fix bottomSheet bottom insets #8331

enahum · 2024-11-12T05:21:50Z

Summary

This PR fixes the all the bottom sheets bottom insets, particularly visible on iOS devices.

Now we don't need to pass the inset to the function or styles but only to the library BottomSheet component as a prop, which also simplifies a lot of code.

Ticket Link

https://mattermost.atlassian.net/browse/MM-55073

Release Note

NONE

cpoile

Nice, and thanks for the fixes

cpoile · 2024-11-12T15:08:11Z

app/products/calls/hooks.ts

@@ -217,7 +217,7 @@ export const useHostMenus = () => {
        const props = {closeButtonId: closeUserProfile, location: '', userId: session.userId};

        openAsBottomSheet({screen, title, theme, closeButtonId: closeUserProfile, props});
-    }, [theme, currentCall?.channelId]);
+    }, [intl, theme]);


lindy65 · 2024-11-14T08:50:38Z

Hi @enahum - please let me know if the screenshots are correct and what is expected

Bottom sheet expanded

Bottom sheet normal

enahum · 2024-11-14T09:43:49Z

@lindy65 yup, that is fine.. you can also try with up to 3 teams and see the difference.

enahum · 2024-11-14T09:44:32Z

@lindy65 btw even if the ticket does not mention it.. this PR is affecting ALL the bottom sheets across the app.

lindy65 · 2024-11-14T11:11:58Z

Hey @enahum

Testing on Android, the first screenshot is first time open from Search and tapping on the Team icon (it doesn't change if I close and reopen)

The second screenshot is if I try to swipe up on the bottom sheet to maximise it - it doesn't change size but rather scrolls the contents of the bottom sheet up

Does this look correct to you? Behavior in the bottom sheet is different to iOS/iPhone as there, the bottom sheet is maximisable when swiping up on it whereas Android the size of the bottom sheet doesn't change but the contents scroll

enahum · 2024-11-14T11:14:40Z

@lindy65 on Android that is not correct.. as I mentioned before please check ALL bottom sheets across the entire app!!!!!!!

That said, in the meantime, I'll check if I get the same results on Android. can you share what version of android are you on?

lindy65 · 2024-11-14T11:17:23Z

Yep, I am testing the PR on all mobile platforms @enahum

I'm on Samsung S22, Android v14

lindy65 · 2024-11-14T11:18:33Z

Ooooh, you mean ALL bottom sheets - not just THIS bottom sheet across platforms - got it now :)

enahum · 2024-11-14T11:22:12Z

Yep, I am testing the PR on all mobile platforms @enahum

I'm on Samsung S22, Android v14

ok that is good,.. but is ALL bottom sheets.. reactions, participants, the plus sign in the home screen, multi-server, three dots in the channel screen, announcement banner, add bookmarks, file search results, long press on at mentions, long press on links, post options, long press on markdown images, pressing on the camera icon when composing a message, post priority, retry a failed post, audio devices for calls, host controls for calls, browse channels (when you tap on switching the channel type), etc..

if you look at the code, the name of the files should give you an idea

enahum · 2024-11-14T13:27:44Z

Hey @enahum

Testing on Android, the first screenshot is first time open from Search and tapping on the Team icon (it doesn't change if I close and reopen)

The second screenshot is if I try to swipe up on the bottom sheet to maximise it - it doesn't change size but rather scrolls the contents of the bottom sheet up

Does this look correct to you? Behavior in the bottom sheet is different to iOS/iPhone as there, the bottom sheet is maximisable when swiping up on it whereas Android the size of the bottom sheet doesn't change but the contents scroll

@lindy65 this one should be fixed with the latest commit.

lindy65 · 2024-11-14T17:52:07Z

Hey @enahum
Testing on Android, the first screenshot is first time open from Search and tapping on the Team icon (it doesn't change if I close and reopen)
The second screenshot is if I try to swipe up on the bottom sheet to maximise it - it doesn't change size but rather scrolls the contents of the bottom sheet up
Does this look correct to you? Behavior in the bottom sheet is different to iOS/iPhone as there, the bottom sheet is maximisable when swiping up on it whereas Android the size of the bottom sheet doesn't change but the contents scroll

@lindy65 this one should be fixed with the latest commit.

Thanks @enahum - this is indeed fixed with the latest commit.

lindy65

Thanks @enahum - I have tested all the bottom sheets according to files changed and they look good to me - I didn't find any issues apart from the issue on Android which you have already fixed.

matthewbirtch

Thanks for looking at this @enahum. A few things I found:

On Android 13, on a Pixel 4, bottom sheets look like they need some more breathing room on the bottom. Here's a few examples below. The black gesture bar at the bottom is affecting the appropriate breathing room. I think it's been like this for a while though. Incidently, not all bottom sheets look like this. For example, the call participants bottom sheet has the right spacing on the bottom.
In the global search, the 'select a team' bottom sheet doesnt' have the correct padding like the other bottom sheets do.
Not sure if this is related to this PR, but I'm not seeing it in the production app. In global search, when I tested searching for a file, the app crashed and went to a white screen. I was unable to test the file options bottom sheet from here. When testing the Channel-specific Files list, it goes white too.
Looks like some additional breathing room is needed on the message priority bottom sheet (on iPhone). Comparing that to the servers bottom sheet which also has s footer button, that screen has the right bottom inset. On iPad the message priority modal footer looks uneven as well.
- Comparison of server bottom sheet and message priority bottom sheet:
- iPad message priority modal
The ••• bottom sheet for an active call seems to have more space than needed compared to other bottom sheets that only have a few options.
Not sure if this is a bottom sheet component affected in this PR, but on iPhone, in the start a new DM modal, when I select users, there seems to be extra space at the bottom of the panel that holds the selected users

enahum · 2024-11-18T15:00:43Z

@matthewbirtch can you provide measurements instead of some space ?

why do I ask, cause everything is the same, just move the bottom inset to another more centralized placed in the code.

matthewbirtch · 2024-11-18T15:58:18Z

👇 add 12 space please on Android

👇 For the message priority bottom sheet, it looks good on iPhone/Android phone now, but the iPad now has too much space in the button footer. It should be reduced for iPad by 12.

👇 For the DM selection bottom container, remove 20 below the button

With the latest update, I just noticed that you fixed the Android search bottom sheet which is great, but on iOS now a scrollbar shows if there are 3 teams. I know anything beyond 3 teams can be visible by expanding the bottom sheet, but I think the scroll bar for 3 teams isn't intended.

ScreenRecording_11-18-2024.10-36-33_1.mov

enahum · 2024-11-20T02:58:29Z

@matthewbirtch changes made :)

matthewbirtch

Looks great @enahum. I'm going to approve. Only one minor update on iPad:

The space at the bottom of the message priority modal is now too small (was too big previously). It should a space of 20, but looks to be 12 on iPad (only on iPad)

enahum · 2024-11-29T04:41:43Z

@matthewbirtch fixed

Fix bottomSheet bottom insets

db57a59

enahum added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester 2: UX Review Requires review by a UX Designer labels Nov 12, 2024

enahum requested review from cpoile, larkox, matthewbirtch and lindy65 November 12, 2024 05:21

mm-cloud-bot added release-note-none labels Nov 12, 2024

lindy65 added the Build Apps for PR Build the mobile app for iOS and Android to test label Nov 12, 2024

larkox approved these changes Nov 12, 2024

View reviewed changes

cpoile approved these changes Nov 12, 2024

View reviewed changes

enahum added Build Apps for PR Build the mobile app for iOS and Android to test and removed 2: Dev Review Requires review by a core commiter Build Apps for PR Build the mobile app for iOS and Android to test labels Nov 13, 2024

fix search team picker on android

5290321

lindy65 added Build Apps for PR Build the mobile app for iOS and Android to test and removed Build Apps for PR Build the mobile app for iOS and Android to test labels Nov 14, 2024

lindy65 approved these changes Nov 14, 2024

View reviewed changes

matthewbirtch requested changes Nov 15, 2024

View reviewed changes

enahum added 2 commits November 20, 2024 09:39

Merge branch 'main' into MM-55073

ee04598

ux review

a96c9dc

enahum requested a review from matthewbirtch November 20, 2024 02:58

enahum removed Build Apps for PR Build the mobile app for iOS and Android to test 3: QA Review Requires review by a QA tester labels Nov 20, 2024

matthewbirtch added the Build Apps for PR Build the mobile app for iOS and Android to test label Nov 22, 2024

matthewbirtch approved these changes Nov 28, 2024

View reviewed changes

enahum added 2 commits November 29, 2024 12:02

Merge branch 'main' into MM-55073

30c7455

fix footer padding for message priority modal on iPad

f1e7ecd

enahum merged commit 7cb2ee7 into main Nov 29, 2024
4 checks passed

enahum deleted the MM-55073 branch November 29, 2024 04:50

amyblais added this to the v2.24.0 milestone Nov 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bottomSheet bottom insets #8331

Fix bottomSheet bottom insets #8331

enahum commented Nov 12, 2024

cpoile left a comment

cpoile Nov 12, 2024

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

lindy65 left a comment

matthewbirtch left a comment

enahum commented Nov 18, 2024

matthewbirtch commented Nov 18, 2024

enahum commented Nov 20, 2024

matthewbirtch left a comment

enahum commented Nov 29, 2024

Fix bottomSheet bottom insets #8331

Fix bottomSheet bottom insets #8331

Conversation

enahum commented Nov 12, 2024

Summary

Ticket Link

Release Note

cpoile left a comment

Choose a reason for hiding this comment

cpoile Nov 12, 2024

Choose a reason for hiding this comment

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

lindy65 commented Nov 14, 2024

enahum commented Nov 14, 2024

enahum commented Nov 14, 2024

lindy65 commented Nov 14, 2024

lindy65 left a comment

Choose a reason for hiding this comment

matthewbirtch left a comment

Choose a reason for hiding this comment

enahum commented Nov 18, 2024

matthewbirtch commented Nov 18, 2024

enahum commented Nov 20, 2024

matthewbirtch left a comment

Choose a reason for hiding this comment

enahum commented Nov 29, 2024