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

MM-54278 - Calls banners redesign #7611

Merged
merged 9 commits into from
Nov 3, 2023
Merged

MM-54278 - Calls banners redesign #7611

merged 9 commits into from
Nov 3, 2023

Conversation

cpoile
Copy link
Member

@cpoile cpoile commented Oct 17, 2023 •
edited
Loading

Summary

  • This PR changes and does a refactor of the calls banner design and UI, partly for polish, partly to accommodate the upcoming bookmarks feature.
  • It also refactors the internal design (to rely more on flex gap instead of manually computing distances and placing banners with absolute positions). This simplifies a lot of the code.
  • The voice-on (talking) shadow has been replaced by the dual border effect for iOS. This makes it look like Android now.
  • The talking borders are a bit smaller in the current call, a bit bigger in the call screen.
  • Emojis were also inconsistent across devices and avatar sizes, made them similar. Also tightened up some of the spacing between the avatars and the names
  • Todo:
    • adjust the New Messages bar for the new UI, and place it under all the banners (after adding the error bars and the incoming call notifications it was hidden).
    • call from another server changes
    • calls banners on the channel list screen

Designs

Ticket Link

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.

Device Information

  • Android: 13, Pixel 6
  • iOS: 16.5.1, iPhone 14

Screenshots

Android: (ios looks the same)

NOTE: the first two call quality banners have the correct styling, imagine the others have it too.

col1 col2 col3
Screenshot_20231017-174950 Screenshot_20231017-172024 Screenshot_20231017-172032
Screenshot_20231017-172055 Screenshot_20231017-175132 Screenshot_20231017-171547
Screenshot_20231017-171431 Screenshot_20231017-171409 Screenshot_20231017-171339
Screenshot_20231017-165254

iOS so you can see that the emojis are a bit bigger now (matches Android), and the voice indicator is the same as Android.

col1 col2
IMG_D899620474FB-1 IMG_85E8F01037AD-1

Release Note

Calls: UI redesign and polish

@cpoile cpoile added 2: Dev Review Requires review by a core commiter 2: UX Review Requires review by a UX Designer labels Oct 17, 2023
@matthewbirtch matthewbirtch added the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 18, 2023
@mattermost-build
Copy link
Contributor

Building app in separate branch.

@mattermost-build mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 18, 2023
matthewbirtch
matthewbirtch requested changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Great work @cpoile. You were very faithful to the designs :) I don't have much to comment on - just these few things.

All banners

  • Are we able to add a shadow to the banners? Or is that not easy to pull off? Designs have a subtle shadow for all banners
  • Also, I'd like to remove the borders on the banners - with the exception of the active call widget (that one needs it for dark themes). However, we can reduce the opacity on the border to 0.08.

Active Call Widget

  • For the 'no one is talking' state, I added a screen in Figma that improves this a little. Could you update that while we're making changes here?
    image

Incoming call banner

  • As discussed in our DM, I seem to be getting an in-app notification overlaid on top of the incoming call
    image

Call quality banner

  • Looks like the x on the call quality banner has to update its color
    image
  • Might need a very slight adjustment for the icon and text to align with other banners. Or do my eyes deceive me?
    image
  • Same goes for the red microphone banner

@cpoile
Copy link
Member Author

cpoile commented Oct 18, 2023

@matthewbirtch Updated:

  • light shadows (matching what we have for the New Messages banner, but I also added some for Android
  • removed borders (except for Current call, and notifications in the Call Screen)
  • made the changes for the 'no one is talking' state
  • the push notification needs to be investigated: https://mattermost.atlassian.net/browse/MM-55036
  • the banners should align now
IMG_F966FD40DFA4-1 IMG_A76FBDE68F8F-1
IMG_1326 IMG_1324

@matthewbirtch
Copy link
Contributor

@matthewbirtch Updated:

  • light shadows (matching what we have for the New Messages banner, but I also added some for Android
  • removed borders (except for Current call, and notifications in the Call Screen)
  • made the changes for the 'no one is talking' state
  • the push notification needs to be investigated: https://mattermost.atlassian.net/browse/MM-55036
  • the banners should align now

Looks good @cpoile. Thanks for making those adjustments.

For the notification issue, I wanted to be clear that it's not actually a push notification that I'm seeing. It's an in-app notification. Those are used when you get notifications from other channels or DMs you're not currently viewing. So, for incoming calls, we should prevent those from showing.

One other thing I just noticed in your screenshots is that the active call banner looks like it has too much padding on the left side now.

image

@enahum
Copy link
Contributor

enahum commented Oct 19, 2023

can you share a screenshot of the user avatar stack on the post list in a channel to see the impact on CRT posts ?

@cpoile
Copy link
Member Author

cpoile commented Oct 19, 2023 •
edited
Loading

@matthewbirtch Good eye, here's the fix (I think it matches; I'm going by the figma distances and using those as the margin/paddings).
IMG_1329 IMG_1330

@enahum Here's the pre (main) vs post (this pr): (edited -- I took new shots, just to make sure I have the right ones)

pre post
IMG_C59C26A5BDE3-1 IMG_0B984261CD0A-1

@matthewbirtch
Copy link
Contributor

@matthewbirtch Good eye, here's the fix (I think it matches; I'm going by the figma distances and using those as the margin/paddings).

IMG_1329 IMG_1330

👍 looks good now @cpoile. Were you going to look into the in-app notifications issue separately?

@cpoile
Copy link
Member Author

cpoile commented Oct 19, 2023

@matthewbirtch

👍 looks good now @cpoile. Were you going to look into the in-app notifications issue separately?

Yep, see above:

@streamer45 streamer45 removed the 2: UX Review Requires review by a UX Designer label Oct 30, 2023
streamer45
streamer45 approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@streamer45 streamer45 left a comment

Choose a reason for hiding this comment

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

Very thorough, thanks 👍

@streamer45 streamer45 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Oct 30, 2023
@cpoile cpoile merged commit a0b1367 into main Nov 3, 2023
@cpoile cpoile deleted the MM-54278-calls-banners-redesign branch November 3, 2023 20:23
@amyblais amyblais added this to the v2.11.0 milestone Nov 3, 2023
fewva pushed a commit to fewva/mattermost-mobile that referenced this pull request Jan 12, 2024
@cpoile
MM-54278 - Calls banners redesign (mattermost#7611)
482ef4e 
* join call banner

* same style of voice-on indicator; avatar & emoji sizing spacing polish

* refactor all calls bars; new calls bars UI designs

* i18n changes

* fix for warning bar's close icon color

* refactor banners to normalize UI; new 'noone is talking' design

* fix for avatar in current call bar

* change design for incoming call on calls screen

* remove commented out code
cyrusjc pushed a commit to cyrusjc/mattermost-mobile that referenced this pull request May 18, 2024
@cpoile @cyrusjc
MM-54278 - Calls banners redesign (mattermost#7611)
b4955d8 
* join call banner

* same style of voice-on indicator; avatar & emoji sizing spacing polish

* refactor all calls bars; new calls bars UI designs

* i18n changes

* fix for warning bar's close icon color

* refactor banners to normalize UI; new 'noone is talking' design

* fix for avatar in current call bar

* change design for incoming call on calls screen

* remove commented out code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enahum enahum enahum approved these changes

@matthewbirtch matthewbirtch matthewbirtch approved these changes

@streamer45 streamer45 streamer45 approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request release-note
Projects
None yet
Milestone
v2.11.0
Development

Successfully merging this pull request may close these issues.
7 participants
@cpoile @mattermost-build @matthewbirtch @enahum @streamer45 @amyblais @mm-cloud-bot