Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

Increase beginningOfChat avatar from 80px to 100px #49435

Open dubielzyk-expensify opened 13 hours ago

dubielzyk-expensify commented 13 hours ago

We've recently decided to simplify the avatar sizes across the app and as a result we want to update the beginningOfChat avatar size from 80px to 100px. Here's what we want:

image

image

Nothing else should change.

cc @Expensify/design

melvin-bot[bot] commented 13 hours ago

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

shawnborton commented 13 hours ago

Nice, thanks for spinning up the issue!

BhuvaneshPatil commented 13 hours ago

Edited by proposal-police: This proposal was edited at 2024-09-19 04:14:41 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Increase beginningOfChat avatar from 80px to 100px

What is the root cause of that problem?

We use LARGE size.

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/home/report/ReportActionItemCreated.tsx#L78-L84

it uses 80px as height and width https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/styles/utils/index.ts#L106

What changes do you think we should make in order to solve the problem?

We need to modify this size, but if we only want to change the size for this particular avatar, we can introduce new key and give it size of 100px.

We can use XLARGE key.

size={isLargeScreenWidth || (icons && icons.length < 3) ? CONST.AVATAR_SIZE.XLARGE : CONST.AVATAR_SIZE.MEDIUM}

If we want to keep XLARGE for all screens

size={CONST.AVATAR_SIZE.XLARGE}

What alternative solutions did you explore? (Optional)

BhuvaneshPatil commented 13 hours ago

@dubielzyk-expensify @shawnborton , In case of multiple avatar (group chat), do we need to change the size as well? or only for single avatar

nkdengineer commented 13 hours ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Increase beginningOfChat avatar from 80px to 100px

What is the root cause of that problem?

Currently we use avatar size is LARGE here

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/home/report/ReportActionItemCreated.tsx#L80

What changes do you think we should make in order to solve the problem?

Simply we should change the LARGE size in here to XLARGE size

size={CONST.AVATAR_SIZE.XLARGE}

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/home/report/ReportActionItemCreated.tsx#L80

What alternative solutions did you explore? (Optional)

nyomanjyotisa commented 13 hours ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Increase beginningOfChat avatar from 80px to 100px

What is the root cause of that problem?

We use large and medium here https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/home/report/ReportActionItemCreated.tsx#L80

What changes do you think we should make in order to solve the problem?

If we only want to change the size to 100px if isLargeScreenWidth or (icons && icons.length < 3) we can change to the following

size={isLargeScreenWidth || (icons && icons.length < 3) ? CONST.AVATAR_SIZE.XLARGE : CONST.AVATAR_SIZE.MEDIUM}

What alternative solutions did you explore? (Optional)

If we want to change the size to 100px on all conditions, we can change to the following

size={CONST.AVATAR_SIZE.XLARGE}

If we want to change the size to 100px if isLargeScreenWidth or (icons && icons.length < 3) and otherwise set the size to 80px, we can change to the following

size={isLargeScreenWidth || (icons && icons.length < 3) ? CONST.AVATAR_SIZE.XLARGE : CONST.AVATAR_SIZE.LARGE}
dubielzyk-expensify commented 12 hours ago

@dubielzyk-expensify @shawnborton , In case of multiple avatar (group chat), do we need to change the size as well? or only for single avatar

Yes, though I believe group chat actually just creates a group avatar now. But yes, all avatars that's in the start of chat area should be 100x100:

image

shawnborton commented 11 hours ago

Yup exactly, new group chats would only have the one group chat avatar, not multiple like they used to. No idea how this impacts older group chats that were created but that won't effect new users on NewDot.

nkdengineer commented 9 hours ago

update proposal following the new requirement here