community-platform
community-platform copied to clipboard
feat: Resize & Crop Member Profile Picture to 400px before upload
PR Checklist
- [x] - Commit messages are descriptive, it will be used in our Release Notes
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.
Passing run #3344 ↗︎
![]() |
![]() |
![]() |
![]() |
![]() |
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.
Could you please upload video of functionality working after change?
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.
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?
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."
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!
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 😬
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 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.
@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.
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.
Been a while here @orditeck, just wondering if you still were interested on working on this issue?
🤖 Closing as stale