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.

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

Workspace - Workspace avatar is different when viewed in full screen mode #41387

Closed
6 tasks done
izarutskaya opened this issue May 1, 2024 · 24 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Weekly KSv2

Comments

@izarutskaya
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.69-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Account settings > Workspaces.
  3. Create a new workspace.
  4. Go to workspace chat.
  5. Click on the chat header.
  6. Click on the workspace avatar.

Expected Result:

Workspace avatar will be the same when viewed in full screen mode.

Actual Result:

Workspace avatar is different when viewed in full screen mode.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6467471_1714543808530.ws_pic.mp4

View all open jobs on GitHub

@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 1, 2024
Copy link

melvin-bot bot commented May 1, 2024

Triggered auto assignment to @sakluger (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.

Copy link

melvin-bot bot commented May 1, 2024

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels May 1, 2024
Copy link
Contributor

github-actions bot commented May 1, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@izarutskaya
Copy link
Author

@sakluger I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@dragnoir
Copy link
Contributor

dragnoir commented May 1, 2024

Proposal

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

Workspace avatar is different when viewed in full screen mode

What is the root cause of that problem?

Offended PR: #39637

The PR updates the logic on how the workspace vatar color is generated. It was generated based on workspace names and now it's based on PolicyID instead of Workspace Name

The new logic is not applied to the avatar inside 1. AttachmentView

const defaultWorkspaceAvatarColor = StyleUtils.getDefaultWorkspaceAvatarColor(file.name ?? '');
iconFillColor = defaultWorkspaceAvatarColor.fill;
additionalStyles = [defaultWorkspaceAvatarColor];
}
return (
<Icon
src={source}
height={variables.defaultAvatarPreviewSize}
width={variables.defaultAvatarPreviewSize}
fill={iconFillColor}
additionalStyles={additionalStyles}
/>
);

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

We shoudl apply the new logic to AttachmentView too.

We can update the defaultWorkspaceAvatarColor and iconFillColor with the new logic here

We replace the file.name bu the policyID

StyleUtils.getDefaultWorkspaceAvatarColor(policyID?.toString() ??  '');

POC video:

20240501_102634.mp4

@cristipaval
Copy link
Contributor

cristipaval commented May 1, 2024

Assigning @grgia and @rushatgabhane as the reviewers of the offending PR

@grgia
Copy link
Contributor

grgia commented May 1, 2024

Another regression @nkdengineer @rushatgabhane #39637

@grgia
Copy link
Contributor

grgia commented May 1, 2024

This one actually isnt a regression, closing

@grgia grgia closed this as completed May 1, 2024
@dragnoir
Copy link
Contributor

dragnoir commented May 1, 2024

@grgia why closing? It's an issue here and a bug that needs to be fixed, I think!

@grgia
Copy link
Contributor

grgia commented May 1, 2024

Not a regression, it's the same on prod @dragnoir

@grgia grgia reopened this May 1, 2024
@grgia
Copy link
Contributor

grgia commented May 1, 2024

@dragnoir my bad I looked at the wrong video 😂

@nkdengineer this should be fixed as a follow up to your PR cc @rushatgabhane

@grgia grgia added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels May 1, 2024
@sakluger
Copy link
Contributor

sakluger commented May 7, 2024

This is being fixed in this PR: #41485

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 7, 2024
@sakluger
Copy link
Contributor

The PR is still in review.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 13, 2024
Copy link

melvin-bot bot commented May 15, 2024

@sakluger @rushatgabhane @grgia this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@sakluger
Copy link
Contributor

PR was merged! Not deployed yet.

@melvin-bot melvin-bot bot removed the Overdue label May 15, 2024
@sakluger sakluger removed their assignment May 16, 2024
@sakluger sakluger added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels May 16, 2024
Copy link

melvin-bot bot commented May 16, 2024

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.

@sakluger sakluger self-assigned this May 16, 2024
@sakluger
Copy link
Contributor

@puneetlath assigning you to help watch this issue while I'm OOO through May 31. Could you please handle payment if needed? The issue was just merged, so it likely will need to be paid out before I return.

I'll for some reason the issue is still open when I return, I'll take it back over.

@melvin-bot melvin-bot bot added the Overdue label May 20, 2024
Copy link

melvin-bot bot commented May 20, 2024

@puneetlath, @sakluger, @rushatgabhane, @grgia Whoops! This issue is 2 days overdue. Let's get this updated quick!

@puneetlath puneetlath added Weekly KSv2 and removed Daily KSv2 labels May 20, 2024
@melvin-bot melvin-bot bot removed the Overdue label May 20, 2024
@rushatgabhane
Copy link
Member

PR merged last week but not on staging yet

@sakluger
Copy link
Contributor

sakluger commented Jun 4, 2024

Removed @puneetlath since I'm back from OOO.

Sounds like we still just need to wait for the PR to be deployed. May be a bit longer with the current freeze.

@melvin-bot melvin-bot bot added the Overdue label Jun 12, 2024
@sakluger
Copy link
Contributor

PR was deployed to prod! Closing.

@melvin-bot melvin-bot bot removed the Overdue label Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Weekly KSv2
Projects
Archived in project
Development

No branches or pull requests

7 participants