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.

Sign up for GitHub

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

Jump to bottom

[HOLD for payment 2023-02-23] [$1000] Figure out why svg images don't show immediately when uploading as avatar #13038

Closed
Beamanator opened this issue Nov 25, 2022 · 106 comments
Closed

[HOLD for payment 2023-02-23] [$1000] Figure out why svg images don't show immediately when uploading as avatar #13038

Beamanator opened this issue Nov 25, 2022 · 106 comments
Assignees
@Beamanator @Ollyws @trjExpensify @Santhosh-Sellavel
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@Beamanator
Copy link
Contributor

Beamanator commented Nov 25, 2022 •
edited
Loading

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:

  1. In main, add 'svg' to the list of allowed avatar extensions in const AVATAR_ALLOWED_EXTENSIONS: ['jpg', 'jpeg', 'png', 'gif', 'bmp'],
  2. On Android or iOS App, navigate to Profile page
  3. Click avatar, choose "Upload photo"
  4. Select a svg image, then continue through the crop modal to upload the image

Expected Result:

User can immediately see the avatar image that they just uploaded

Actual Result:

  • Image doesn't show up until it uploads to the server and the URL is returned
  • User needs to refresh to see the image

Workaround:

None needed

Platform:

Where is this issue occurring?

  • All

Version Number:
Reproducible in staging?: Y
Reproducible in production?: Y
GH conversation: #12549 (comment)

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01953030ea34ada712
  • Upwork Job ID: 1622513437122138112
  • Last Price Increase: 2023-02-06
@Beamanator Beamanator added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 25, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 25, 2022

Triggered auto assignment to @arielgreen (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@Beamanator Beamanator changed the title Allow bmp & gif avatar uploads on Nav Apps Allow bmp & gif avatar uploads on Native Apps Nov 25, 2022
@Beamanator Beamanator added Engineering Weekly KSv2 and removed Daily KSv2 labels Nov 25, 2022
@Beamanator Beamanator changed the title Allow bmp & gif avatar uploads on Native Apps [HOLD #12549] Allow bmp & gif avatar uploads on Native Apps Nov 25, 2022
@Beamanator Beamanator mentioned this issue Nov 25, 2022
Closed
@Beamanator
Copy link
Contributor Author

While this issue currently exists, I would prefer waiting till #12549 gets merged before working on this (most likely can be external) since in that PR we're enabling other image formats to be uploaded

@Beamanator Beamanator mentioned this issue Nov 25, 2022
Merged
92 tasks
@Beamanator Beamanator changed the title [HOLD #12549] Allow bmp & gif avatar uploads on Native Apps [HOLD #11063] Allow bmp & gif avatar uploads on Native Apps Dec 2, 2022
@Beamanator
Copy link
Contributor Author

#12549 just got merged! I thinkkkkk we should still wait for it to get to Production, just in case there's any regressions found

@Beamanator Beamanator mentioned this issue Dec 2, 2022
Closed
39 tasks
@JmillsExpensify
Copy link

JmillsExpensify commented Dec 8, 2022 •
edited
Loading

Additionally, now that #12549 just hit production, we should remove the hold. Woo!

@JmillsExpensify
Copy link

cc @trjExpensify Since you're on the main tracking issue.

@trjExpensify
Copy link
Contributor

Ah yeah, let me take this over for you @arielgreen. :)

@trjExpensify
Copy link
Contributor

@Beamanator what's the plan? Are you taking this or are we going external? I agree it should come off hold now.

@Beamanator Beamanator changed the title [HOLD #11063] Allow bmp & gif avatar uploads on Native Apps Allow bmp & gif avatar uploads on Native Apps Dec 13, 2022
@Beamanator Beamanator added the Planning Changes still in the thought process label Dec 13, 2022
@Beamanator Beamanator mentioned this issue Dec 13, 2022
Closed
@Beamanator Beamanator changed the title Allow bmp & gif avatar uploads on Native Apps Allow bmp & gif avatar uploads on Native Apps & svg everywhere Dec 13, 2022
@melvin-bot melvin-bot bot added the Overdue label Dec 16, 2022
@trjExpensify
Copy link
Contributor

@Beamanator, this issue will hit 4 weeks old after next week and it doesn't seem like we've made any further progress here yet to get to a resolution before then. What are the next steps?

@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 16, 2023
@MelvinBot
Copy link

MelvinBot commented Feb 16, 2023 •
edited by trjExpensify
Loading

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.72-1 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-02-23. 🎊

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.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@adelekennedy adelekennedy mentioned this issue Feb 23, 2023
Closed
6 tasks
@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Feb 23, 2023
@trjExpensify
Copy link
Contributor

Did @adelekennedy pick up a regression that's linked there or is this good to be released for payment?

@melvin-bot melvin-bot bot removed the Overdue label Feb 27, 2023
@Ollyws
Copy link
Contributor

Ollyws commented Mar 1, 2023

@trjExpensify I tested extensively with many different SVG files, including the file used in #15189 and couldn't reproduce the issue.

Any thoughts @Beamanator @Santhosh-Sellavel ?

@Beamanator
Copy link
Contributor Author

Yeah and I just tested with an SVG and it worked fine 🤔

@kbecciv can you please test uploading an SVG again, like you did in this issue: #15189

And if you are unable to upload, please share the SVG image that failed to upload

@kbecciv
Copy link

kbecciv commented Mar 2, 2023 •
edited
Loading

@Beamanator Please attach a new video and SVG file per your request.

Record_2023-03-02-17-34-47.mp4

svg_logo (3)

@Ollyws
Copy link
Contributor

Ollyws commented Mar 2, 2023 •
edited
Loading

@kbecciv It seems like the SVG file you are using is actually a png file with the wrong extension. Hence the problem uploading.
Perhaps try testing with this one.

@Santhosh-Sellavel
Copy link
Collaborator

Yes, it's not a valid SVG. @Beamanator

@Beamanator
Copy link
Contributor Author

Aah interesting, how do y'all know it's actually a PNG image? 😅 Anyway, I think we're good to go here, no regressions @trjExpensify 👍

@Ollyws
Copy link
Contributor

Ollyws commented Mar 2, 2023

@Beamanator Just used the file command on *nix.

@Santhosh-Sellavel
Copy link
Collaborator

@Beamanator
I just tried opening it in a VSCode but it's not opening. SVG images are XML based and should be able to view/edit in VSCode (any text editor)

@trjExpensify
Copy link
Contributor

Okay great, thanks for confirming! Offers for $1,000 have been sent to you both.

@Ollyws
Copy link
Contributor

Ollyws commented Mar 3, 2023

@trjExpensify Accepted, thanks!

@trjExpensify
Copy link
Contributor

Settled up with @Ollyws! Over to you, @Santhosh-Sellavel.

@Ollyws
Copy link
Contributor

Ollyws commented Mar 3, 2023 •
edited
Loading

@trjExpensify I believe we're eligible for the timeline bonus, as I was assigned on Friday Feb 10th, and it was merged on Tuesday Feb 14th. Thanks!

@trjExpensify
Copy link
Contributor

Ah shit, that was the weekend. Yes, you're right! I've paid you $500 as a bonus.

@trjExpensify
Copy link
Contributor

@Santhosh-Sellavel after you accept, I'll modify to $1,500.

@melvin-bot melvin-bot bot added the Overdue label Mar 6, 2023
@Beamanator
Copy link
Contributor Author

Seems we're only waiting on @Santhosh-Sellavel to accept the Upwork job, then we can pay out & close

@melvin-bot melvin-bot bot removed the Overdue label Mar 6, 2023
@trjExpensify
Copy link
Contributor

Yep, correct!

@trjExpensify
Copy link
Contributor

Settled up with @Santhosh-Sellavel. Closing!

@MitchExpensify MitchExpensify mentioned this issue Mar 18, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Beamanator Beamanator

@Ollyws Ollyws

@trjExpensify trjExpensify

@Santhosh-Sellavel Santhosh-Sellavel

Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests