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

[Invoice Rooms] Avatar not matching from InvoiceRoom header to InvoiceRoom invoice preview #41473

Closed
danielrvidal opened this issue May 2, 2024 · 16 comments
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@danielrvidal
Copy link
Contributor

If you look at the header on this invoice room and then the avatar on report preview, you can see the C is green in the header and then blue on the invoice preview. I have reproduced this.

image

@danielrvidal danielrvidal added External Added to denote the issue can be worked on by a contributor Daily KSv2 labels May 2, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 2, 2024
Copy link

melvin-bot bot commented May 2, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

@danielrvidal danielrvidal changed the title Avatar not matching from InvoiceRoom header to InvoiceRoom invoice preview [Invoice Rooms] Avatar not matching from InvoiceRoom header to InvoiceRoom invoice preview May 2, 2024
@danielrvidal
Copy link
Contributor Author

Just for reference, it might have something to do with this: #41316

@cretadn22
Copy link
Contributor

Proposal

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

The background of default avatar is inconsistency

What is the root cause of that problem?

At present, within our application, we create the standard workspace avatar using the policyID to ensure uniformity. But in ReportActionItemSingle Component

id: isWorkspaceActor ? '' : actorAccountID,

If isWorkspaceActor is true, we don't include the policyID, resulting in a different background color for the avatar compared to other instances where the policyID is used to determine the background color.

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

id: isWorkspaceActor ? '' : actorAccountID,

We need to pass report.policyID if isWorkspaceActor is true

We also need to address the same issue in the tooltips.

Screenshot 2024-05-02 at 12 30 33

We need to pass accountID={userAccountID} params here

We additionally require updating the type of the accountID props in the UserDetailsTooltip Component to number | string since the policyID might not be exclusively a number. Alternatively, we can introduce new props like avatarID if we prefer not to modify the type of existing props.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@Nodebrute
Copy link
Contributor

Regression from #39637

@thesahindia
Copy link
Member

Regression from #39637

@rushatgabhane @nkdengineer, can you guys confirm this?

@nkdengineer
Copy link
Contributor

@thesahindia This will be fixed in this PR #41485.

@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors labels May 6, 2024
@cristipaval cristipaval assigned grgia and unassigned thesahindia May 6, 2024
@melvin-bot melvin-bot bot added the Overdue label May 8, 2024
Copy link

melvin-bot bot commented May 9, 2024

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

@grgia
Copy link
Contributor

grgia commented May 13, 2024

PR still in review

@melvin-bot melvin-bot bot removed the Overdue label May 13, 2024
@grgia grgia added the Reviewing Has a PR in review label May 13, 2024
Copy link

melvin-bot bot commented May 20, 2024

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

@rushatgabhane
Copy link
Member

PR merged but not on staging yet

Copy link

melvin-bot bot commented May 28, 2024

@rushatgabhane, @grgia, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented May 29, 2024

@rushatgabhane, @grgia, @nkdengineer Eep! 4 days overdue now. Issues have feelings too...

@davidcardoza
Copy link
Contributor

I believe this was deployed to production here - #41485 can someone confirm?

Copy link

melvin-bot bot commented May 30, 2024

@rushatgabhane, @grgia, @nkdengineer Huh... This is 4 days overdue. Who can take care of this?

@rushatgabhane
Copy link
Member

Yepp this is in production

@davidcardoza
Copy link
Contributor

Closing this issue out, feel free to reopen if needed @cristipaval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

8 participants