-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$1000] Cursor appears zoom cursor for a moment and changes to normal cursor when opening profile pic from details page #16100
Comments
Triggered auto assignment to @greg-schroeder ( |
Bug0 Triage Checklist (Main S/O)
|
@greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Back from OOO, reviewing |
@dhanashree-sawant @kavimuru - I'm a little unclear on the expected behavior here. When is that zoom icon supposed to show up? Is it possible that it should be showing up and remaining when mousing over the photo? Or should it just be the generic cursor icon at all times? Can you please clarify? |
Hi @greg-schroeder, cursor should be generic when we click on attachment and it should display as zoom cursor only when we hover on the image part in the attachment preview. Mac safari does this task right, you can use that as reference point. |
Job added to Upwork: https://www.upwork.com/jobs/~019c8aa50007c703f7 |
Okay, got it! Updated OP with that clarification. Applied |
Triggered auto assignment to @isabelastisser ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif ( |
Triggered auto assignment to @grgia ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.When clicked profile pic, cursor appears zoom cursor for a moment and changes to normal cursor when opening profile pic from details page. What is the root cause of that problem?When we click profile image it will open AttachmentModel >> AttachmentView >> ImageView. App/src/components/ImageView/index.js Lines 275 to 282 in c803b5e
Here is the code for Lines 180 to 188 in c803b5e
This function receive two params So actually what happen here, when So during that fraction of time (i.e. until image dimension decided) it will show What changes do you think we should make in order to solve the problem?Within <Pressable
style={{
...
// ...StyleUtils.getZoomCursorStyle(this.state.isZoomed, this.state.isDragging), // *** OLD CODE ***
...StyleUtils.getZoomCursorStyle(this.state.isZoomed, this.state.isDragging, this.state.imgWidth, this.state.imgHeight), // *** UPDATED CODE ***
...
}} Within function getZoomCursorStyle(isZoomed, isDragging, imgWidth, imgHeight) {
// Add below if condition
if (imgWidth === 0 || imgHeight === 0) {
return {cursor: 'default'};
}
if (!isZoomed) {
return {cursor: 'zoom-in'};
}
return {
cursor: isDragging ? 'grabbing' : 'zoom-out',
};
} So this will solve the problem, we can see the results. What alternative solutions did you explore? (Optional)None ResultImage-open-cursor.mov |
@aimane-chnaif could you take a look at this proposal if you get a chance? |
@greg-schroeder @grgia @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@greg-schroeder, @grgia, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Issue not reproducible during KI retests. (First week) |
@aimane-chnaif let's make sure there's a separate issue for the flicker effect. We can then close out this issue. |
ok, let's close. I just found out that issue - #16528 (comment) |
Hi @grgia , @aimane-chnaif if closed, will it be eligible for reporting bonus? |
From the Contributing Guide:
It looks like the issue that the PR fixed links back to an issue from 2022, so I don't believe this is eligible for the reporting bonus unless you can prove that your bug report came first. cc @greg-schroeder if you have any input on this |
Hi @grgia , sounds fair, thanks for the reply |
You are correct @grgia |
Closing per discussion above |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
App should display image with normal cursor
From issue reporter:
Actual Result:
App first displays image with zoom cursor and after some time changes the cursor back to normal cursor
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.2.87-0
Reproducible in staging?: y
Reproducible in production?: y
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:
Recording.1731.mp4
cursor.fluctate.issue.mov
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679128979658119
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: