community-platform icon indicating copy to clipboard operation
community-platform copied to clipboard

feat: Resize & Crop Member Profile Picture to 400px before upload

Open orditeck opened this issue 1 year ago • 11 comments

PR Checklist

PR Type

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Developer experience (improves developer workflows for contributing to the project)

Description

This PR resizes members' profile picture to a 400x400 pixel square on the client-side (browser) before uploading, reducing unnecessary large files from being stored on the server.

Tophat instructions

You can tophat the changes here:

  • https://community-platform.code.nevek.co

Git Issues

Closes #1589

Screenshots/Videos

If useful, provide screenshot or capture to highlight main changes


What happens next?

Thanks for the contribution! We try to make sure all PRs are reviewed ahead of a monthly dev call (first Monday of the month, open to all!).

If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.

If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.

orditeck avatar Apr 25 '23 14:04 orditeck

Passing run #3344 ↗︎

0 82 0 0 Flakiness 0

Details:

feat: resize & crop Member Profile Picture to 400px before upload
Project: onearmy-community-platform Commit: 4e1ab8e541
Status: Passed Duration: 03:48 💡
Started: Apr 25, 2023 2:43 PM Ended: Apr 25, 2023 2:47 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

cypress[bot] avatar Apr 25 '23 14:04 cypress[bot]

Could you please upload video of functionality working after change?

cerkiewny avatar Apr 25 '23 16:04 cerkiewny

Could you please upload video of functionality working after change?

You can clone & test it locally. I also provided a link to test/tophat the changes.

orditeck avatar Apr 25 '23 16:04 orditeck

Thanks for raising @orditeck, this is great! One of themes we are working on at the moment is improving test coverage across the Platform. As the system continues to become more complicated we are missing out on automated test suites to lower the maintenance costs.

How would you suggest validating this behaviour?

thisislawatts avatar Apr 25 '23 16:04 thisislawatts

Also thanks for introducing me to the tophat terminology. I had not encountered it before:

When someone says "you can tophat the changes here," they are likely using "tophat" as a slang term that means "to quickly or easily make changes or adjustments." It's a colloquialism that is sometimes used in informal or casual settings.

For example, if someone is making edits to a document or presentation and they want to make a quick adjustment, they might say "let me just tophat this real quick" as a way of expressing that they can make the change easily and quickly.

It's worth noting that this usage of "tophat" is not widely recognized or commonly used outside of certain social groups or contexts, so it's possible that not everyone will understand what is meant by the phrase "you can tophat the changes here."

thisislawatts avatar Apr 25 '23 17:04 thisislawatts

Thanks @thisislawatts, looks like chatgpt put in some work, I was quite confused by this term as well. Curious where @orditeck has pulled this out of!

AlfonsoGhislieri avatar Apr 25 '23 17:04 AlfonsoGhislieri

Thanks @thisislawatts, looks like chatgpt put in some work, I was quite confused by this term as well. Curious where @orditeck has pulled this out of!

Haha oops!! I use so commonly at work that I didn't even think twice about it 😬

orditeck avatar Apr 25 '23 17:04 orditeck

Thanks for raising @orditeck, this is great! One of themes we are working on at the moment is improving test coverage across the Platform. As the system continues to become more complicated we are missing out on automated test suites to lower the maintenance costs.

How would you suggest validating this behaviour?

That's an awesome goal! I'll do my best at adding a test, and then we can discuss about the approach I took and what could be better. I looked at a few existing ones and we should probably come with a small set of utils that we could re-use to mount the components in a given context. I saw a lot of repetitive bootstrapping code in the few test files that I checked.

orditeck avatar Apr 25 '23 17:04 orditeck

@orditeck did you look at Uppy to provide the cropping functionality? I see in their demos they have a whole image editing capability. The only caveat here would be that we are trailing a little way behind the latest version.

thisislawatts avatar Apr 26 '23 13:04 thisislawatts

@orditeck did you look at Uppy to provide the cropping functionality? I see in their demos they have a whole image editing capability. The only caveat here would be that we are trailing a little way behind the latest version.

No. I can take a look for sure! My intention with this PR (I could've mentioned that) is to provide a MVP that fits the specs:

  • Resize avatars to a 400px square to save bandwidth (and, all things being equal, save precious Earth resources on the long term)

I'd consider having a customizable cropping experience on the user end a nice-to-have that could be tackled in another PR, but I'm sure there are other priorities (eg, adding tests). Also, I don't know how much we care about the build size, but as a side note, adding more functionalities often means heavier dependencies, leading to a larger build size, leading to slower loading time and responsiveness.

orditeck avatar Apr 26 '23 13:04 orditeck

Thanks for flagging these concerns, all valid. I don't mean to add extra work here but as I was looking elsewhere in the codebase today I rediscovered we already have Uppy available as a dependency. In which case it seems preferable to standardise on its usage rather than introduce new pkg.

thisislawatts avatar Apr 26 '23 15:04 thisislawatts

Been a while here @orditeck, just wondering if you still were interested on working on this issue?

AlfonsoGhislieri avatar Jun 26 '23 13:06 AlfonsoGhislieri

🤖 Closing as stale

thisislawatts avatar Jul 03 '23 19:07 thisislawatts