Closed lanitochka17 closed 10 months ago
Triggered auto assignment to @laurenreidexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Platforms
in OP are β
)Job added to Upwork: https://www.upwork.com/jobs/~0181b4bbc15e998db5
Current assignee @laurenreidexpensify is eligible for the External assigner, not assigning anyone new.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @narefyev91 (External
)
Long-pressing the LHN item will make the second avatar transparent. This happens on the search page too.
Each item in the list uses PressableWithFeedback which has a default pressDimmingValue
of 0.8. Holding down the item will reduce the opacity to 0.8. On Android, having components stacked on top of each other with reduced opacity will not render it correctly. This is the same issue as this one which I explained the issue here or this one.
The solution is to use needsOffscreenAlphaCompositing
on the component that has the opacity applied, which is OpacityView in this case.
Details:
We will apply this to both PressableWithSecondaryInteraction
of
OptionRowLHN
and PressableWithFeedback
of OptionRow
. The needsOffscreenAlphaCompositing
will be true only if the icon's length is >= 2.
PressableWithSecondaryInteraction
uses PressableWithFeedback
internally, so we will pass over the needsOffscreenAlphaCompositing
props to PressableWithFeedback
.
In PressableWithFeedback
, we will pass the needsOffscreenAlphaCompositing
props to OpacityView
, and in OpacityView
, pass the needsOffscreenAlphaCompositing
props to Animated.View
with the added condition of && shouldRenderOffscreen
like we do in OfflineWithFeedback
.
https://github.com/Expensify/App/blob/95c9e4743f44ca42ece2c6ac56259b911d5feafc/src/components/OfflineWithFeedback.js#L117
<OptionRowLHN
<PressableWithSecondaryInteraction needsOffscreenAlphaCompositing={props.optionItem.icons.length >= 2}
<PressableWIthFeedback needsOffscreenAlphaCompositing={props.needsOffscreenAlphaCompositing}
<OptionRow
<PressableWithFeedback needsOffscreenAlphaCompositing={this.props.option.icons.length >= 2}
<PressableWithFeedback
<OpacityView needsOffscreenAlphaCompositing={props.needsOffscreenAlphaCompositing}
<Animated.View needsOffscreenAlphaCompositing={shouldRenderOffscreen && props.needsOffscreenAlphaCompositing}
Proposal from @bernhardoj looks good to me https://github.com/Expensify/App/issues/27396#issuecomment-1719637570 thanks for detail explanation! π π π C+ reviewed
Triggered auto assignment to @MonilBhavsar, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@MonilBhavsar bump ^^
Sorry been dealing with a fire. Taking a look now
Proposal looks good to me. Looks like we have seen similar issues with only Android in the past. If we can add some test steps to prevent it from happening
π£ @bernhardoj π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
PR is ready
cc: @narefyev91
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one π
@bernhardoj Seems regression of crashes after this PR merged. https://expensify.slack.com/archives/C049HHMV9SM/p1695796696023189
@Pujan92 you're right. Not every OptionRow usage has icons. I have raised a new PR to fix it here.
Thanks for letting us know!
cc: @narefyev91 @MonilBhavsar
Thank you!
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one π
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.75-12 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-10-09. :confetti_ball:
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:
Payment Summary:
@narefyev91 please advise on checklist above thanks so we can close this out
Bump for checklist @narefyev91
Payment Summary:
- External issue reporter - Applause N/A
- Contributor that fixed the issue - @narefyev91 expert contributor N/A
- Contributor+ that helped on the issue and/or PR - @narefyev91 = payment issued in Upwork $500
@narefyev91 please advise on checklist above thanks so we can close this out
@laurenreidexpensify contributor that fixed the issue should be @bernhardoj ?
There was regression - https://github.com/Expensify/App/issues/28317
https://github.com/Expensify/App/issues/27396#issuecomment-1761498799
@MonilBhavsar sorry typo! corrected
Looks like we're good to close this issue?
@MonilBhavsar - can you confirm is this is correct - https://github.com/Expensify/App/issues/28317
if no regression feel free to close
@laurenreidexpensify yes, there was a regression coming from this PR which was fixed by @bernhardoj
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
Precondition: User has a workspace IOU and a thread in room
Expected Result:
There is no visual issues with the avatars
Actual Result:
When long pressing on workspace IOU row in LHN, a circular overlay is seen instead of square overlay over the workspace avatar. As for the message thread, the user avatar overlaps with workspace avatar
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.69-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
https://github.com/Expensify/App/assets/78819774/2fc43252-9512-45b3-916b-a433d98b28a3
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit