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.

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

Refactor: Image upload #3270

Open
benfurber opened this issue Feb 12, 2024 · 3 comments
Open

Refactor: Image upload #3270

benfurber opened this issue Feb 12, 2024 · 3 comments

Comments

@benfurber
Copy link
Member

benfurber commented Feb 12, 2024

On the user Settings page (SettingsPage), the user profile image component in UserInfosSection uses ImageInputField which in turn uses ImageInput which has a lot going on in it!

We’re looking to add a feature so that users can select a crop of the image they select but due to the current architecture, this is difficult to implement.

Task

Change ImageInputField so that it is more responsible for:

  1. Determining if an image is empty and thus renders a more simple ‘Upload image’ button.
  2. If the user has selected an image, present the user with the CropImage component (Feat: Add crop image component #3269).
  3. If the user has selected a crop for the image, begin uploading the cropped image and thus rendering a Loader.
  4. If errored, present that error to the user.
  5. If successful, render the uploaded image to the user with a delete button beneath - Not an overlay, it’s bad UX for mobile.

Notes:

  • This is more complex for the multi-image implementation for the multi-image implementations of the image upload feature used for workspaces, etc. Don’t worry about that for the moment, let’s get it right for single image use.
  • Please move as much responsibility to the component library as possible.
@manacespereira
Copy link
Contributor

Can i work on this one?

@thisislawatts
Copy link
Collaborator

@manacespereira it looks like @benfurber has started work on this item in #3287

@benfurber
Copy link
Member Author

benfurber commented Feb 22, 2024

Hi @manacespereira. I’d be delighted for you to pick this up.

As @thisislawatts says, there’s #3287 - which doesn’t change the functionality (as this issue proposes) but was meant to be a tidy up to support this ticket. But as you can see from the failing tests, I eeerrr, must have broken something.

Early next week I can finish #3287 and get confirmed that #3269 is the direction we want to travel.

As I want to move as much functionality to the component library as possible you could probably start playing around/architecting/planning this issue now and we have a chat next week about details?

How does that sound?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New / Under Discussion
Development

No branches or pull requests

3 participants