-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2023-08-24] [$1000] Task - Second avatar of group avatar is highlighted when long pressing on Share somewhere #24008
Comments
Triggered auto assignment to @lschurr ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Second avatar of group avatar is highlighted on long press What is the root cause of that problem?MultipleAvatars style ignore App/src/components/MenuItem.js Lines 155 to 156 in 9dc5f19
What changes do you think we should make in order to solve the problem?Check pressed && props.interactive ? StyleUtils.getBackgroundAndBorderStyle(themeColors.buttonPressedBG) : undefined,
isHovered && !pressed && props.interactive ? StyleUtils.getBackgroundAndBorderStyle(themeColors.border) : undefined, What alternative solutions did you explore? (Optional) |
Job added to Upwork: https://www.upwork.com/jobs/~01a8af40e296417c50 |
Current assignee @lschurr is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane ( |
I can do this |
📣 @hanzalahsamana! 📣
|
In components, MenuItem.js, lines 148-158: The pressable this is a child to does check for props.disabled and !props.interactive, but isHovered is passed from further up the hierarchy, so it appears that these checks are not actually diverting inherited values. The first thing I'd do is add in conditionals to isHovered and isPressed that switch to a hardcoded value pending a check to either props.disabled or !props.interactive. there are a couple of different configurations possible there, so I'd try out all of them and log values to see if this theory is correct. |
📣 @icarus-0520! 📣
|
Contributor details |
ProposalPlease re-state the problem that we are trying to solve in this issue.Second avatar of group avatar is highlighted when long pressing on Share somewhere What is the root cause of that problem?MultipleAvatars style ignore props.interactive so the element always highlights the second avatar on press What changes do you think we should make in order to solve the problem?props.interactive should be used to determine if the second avatar should be highlighted on press or not. |
@lanitochka17 Can you please update me as the reporter, I already reported it here: |
ProposalPlease re-state the problem that we are trying to solve in this issue.Secondary avatar of group avatar is highlighted when hovering or long pressing on share somewhere What is the root cause of that problem?When we assign a task in a group chat, props.task.parentReportID is the reportID of the group chat and props.interactive is false in order to prevent interaction App/src/pages/tasks/NewTaskPage.js Line 181 in 6a38341
But Multiple avatar ignores this props App/src/components/MenuItem.js Lines 153 to 157 in 6a38341
This is the root cause What changes do you think we should make in order to solve the problem?This issue may happens for all child components related to We need to replace all occurrence of What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Second avatar of group avatar is highlighted on long press What is the root cause of that problem?When we assign a task in a group chat, the props.task.parentReportID is id of this room. So the App/src/pages/tasks/NewTaskPage.js Line 186 in 9dc5f19
So MultipleAvatars ignore props.interactive and second avatar of group avatar is highlighted when long pressing.
App/src/components/MenuItem.js Lines 155 to 156 in 9dc5f19
What changes do you think we should make in order to solve the problem?Add
What alternative solutions did you explore? (Optional) |
📣 @CuongXuanLe! 📣
|
|
|
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
ProposalPlease re-state the problem that we are trying to solve in this issue.Second avatar of group avatar is highlighted on long press on Share somewhere field What is the root cause of that problem?When the "Share everywhere" menu is pressed, the interactive props are set to false to prevent the avatars in this menu from responding to press events. However, the MultipleAvatars component did not handle this interactive props variable. What changes do you think we should make in order to solve the problem?In the MenuItem.js file, on the 148th line, you can update the use case for the MultipleAvatars component as the following.
What alternative solutions did you explore? (Optional)N/A |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.54-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-08-24. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Hi @sobitneupane - could you work on the checklist? Do we need a regression test for this? |
Payment summary:
|
@lschurr Can you update the reporter? I actually reported this before. |
@sobitneupane could you work through the checklist for this one? |
Could you apply to the job @hungvu193? https://www.upwork.com/jobs/~01a8af40e296417c50 |
Applied, Thank you |
@danieldoglas, @reinaldosg, @sobitneupane, @lschurr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Sorry for the delay. I will try to get to it by EOD. |
Bump on this one @sobitneupane :) |
Hi @sobitneupane - would really like to get this one closed out. Could you confirm whether we need a regression test or not? |
Regression Test Proposal
Do we agree 👍 or 👎 |
https://github.com/Expensify/App/pull/20145/files#r1311937536
Yes
|
@danieldoglas, @reinaldosg, @sobitneupane, @lschurr Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@lschurr I think this one is ready to pay/close! |
Yep, thanks! I'm going to create a QA GH and will close. |
All done! |
Requested payment on newDot. |
$1,500 approved for payment via NewDot based on BZ summary. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Since Share somewhere field is not selectable in group chat task assignment, it should not have any feedback when pressing on it
Actual Result:
The second avatar of group avatar is highlighted when long pressing on Share somewhere
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.48.0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Bug6149411_Screen_Recording_20230801_200354_New_Expensify.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause -Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: