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

Fix/image inputs #958

Merged
merged 7 commits into from
Apr 23, 2020
Merged

Fix/image inputs #958

merged 7 commits into from
Apr 23, 2020

Conversation

chrismclarke
Copy link
Member

@chrismclarke chrismclarke commented Apr 22, 2020

Done

  • Add support for ImageInput to take a generic value property instead of just a preview string (tidies the handling of multiple inputs away from components using the input to the input itself, and makes it possible to do more with the full input meta)
  • Bind ImageInputField to direclty pass value from form (removes need for manual methods)
  • Remove canDelete and replaceImage prop (assume images can always be deleted in edit mode so better to calculate from whether images are present, image replacement nice but not really necessary as user can just delete and then upload a new one)
  • Cherry-picked a few of the fixes from Refactor - CI Backend (+db prefixes) #952 to try and get tests passing
  • Add up arrow as hotfix for howto step arrow display

Todo

  • Now that full upload image meta available we should add a method to delete image from server on image delete (included as inline todo comment, but could be made higher priority?)
  • Not likely required at this point as we don't have anything using multiple image uploads, but if we ever do would probably want to add a different method to display. At the same time we could refactor current forms to use a single multiple image upload field instead of multiple individual ones (v. low priority).
  • After Refactor - CI Backend (+db prefixes) #952 approval will need to resolve conflicts created here and uncomment broken howto image check.

@cypress
Copy link

cypress bot commented Apr 22, 2020



Test summary

30 0 0 0


Run details

Project onearmy-community-platform
Status Passed
Commit 9759f5f ℹ️
Started Apr 22, 2020 9:46 PM
Ended Apr 22, 2020 9:53 PM
Duration 06:33 💡
OS Linux Ubuntu Linux - 14.04
Browser Electron 78

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@BenGamma BenGamma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refacto !
No problem with this code so I'm approving.
One thing to note though is that I fall on bug #781 while trying to upload cover image, I had to change it to make the upload works.
See my comment in issue for potential fix.

@BenGamma BenGamma merged commit ebd5c1c into master Apr 23, 2020
@BenGamma BenGamma deleted the fix/image-inputs branch April 23, 2020 10:16
@BenGamma BenGamma mentioned this pull request Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants