mattermost / mattermost-mobile

Next generation iOS and Android apps for Mattermost in React Native
https://about.mattermost.com/
Apache License 2.0
2.26k stars 1.37k forks source link

fix(MM-49742): broken user avatar #8356

Open rahimrahman opened 6 days ago

rahimrahman commented 6 days ago

Summary

Blank user avatar "hides" menu option.

Ticket Link

Fixes: https://mattermost.atlassian.net/browse/MM-49742

Checklist

Device Information

This PR was tested on: iPhone 15 Simulator, iOS 17.4.1

Screenshots

The profile photo would also show background image with opacity of 12%. Simulator Screenshot - iPhone 15 - 2024-11-18 at 13 04 51

I removed the status icon just to showcase what it would look like. Simulator Screenshot - iPhone 15 - 2024-11-18 at 13 06 22

Simulator Screenshot - iPhone 15 - 2024-11-18 at 13 22 13

Release Note

rahimrahman commented 5 days ago

I will fix test as soon as I know that this is the right fix.

matthewbirtch commented 5 days ago

@rahimrahman are we able to conditionally change the background color? The reason I ask is that it doesn't show up well in all themes on the profile screen. See screenshot below for example.

If it appears on sidebarBg, then we should use 12% sidebarText. If it appears on the centerChannelBg, then it should be 12% centerChannelColor (as you have it).

Is this possible?

rahimrahman commented 4 days ago

@matthewbirtch the biggest challenge here is the backgroundcolor for the Image is buried 2-3 children deep.

(GrandParentComponent)
  |
  - ParentComponent
    |
    - ProfilePicture
      |
      - Image

And the Image component is where I'm setting the background. ParentComponent or GrandParentComponent is the one with the backgroundColor for the entire container, so in order for the Image component to distinguish what backgroundColor is being used from the ParentComponent or GrandParentComponent, we would have to pass the prop down a few levels (prop drilling).

I won't be able to use React context either since changing the backgroundColor based on the ParentComponent or GrandParentComponent will cause changes to all profile images.

What if we do what Elias suggested?

We can add a border to the avatar even if it has an image or a blank image, but I don’t know if that is what we want.

This would isolate the issue only on tab bar, but understandable, this might still be an issue in post, etc.

With Photo Without Photo
withphoto-selected empty-selected
withphoto-unselected empty-unselected