-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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
Add profile image upload component #5355
Conversation
@ErisDS Currently the gravatar part works. There's a couple of issues I ran into with the file upload though. First, upload requests have been returning 401 errors, resulting in the file uploader not working, meaning the uploads api needs to be refactored to allow image uploads for setup, or some other workaround needs to be determined. Second, to trigger the file upload, there needs to be a |
Needing the file input is something you'd likely need @JohnONolan's help with - it'll likely need to be placed where the upload icon With regard to the upload failing, I think we're going to need to do some fancy handling of what-happens-when here. If we use the preview feature of blueimp to display the image, and then send the image with the setup that could work? That would need the setup API endpoint to be adjusted to accept that image I guess? |
export default Ember.Component.extend({ | ||
email: '', | ||
size: 90, | ||
hasUploadedImage: true, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@ErisDS Couldn't the form send the setup form first (through the changes in #5359), and then send the upload image request? IMO that would be simpler and easier than trying to attach the image to the json post request. It would reduce the needed api modifications as well. Also if this is planned to be a re-usable component within the application (which would mean the user would be logged in), it would POST to the api/uploads route anyways. |
That sounds perfectly sensible 👍 |
Ditto here - is there anything else you need from me on this one? |
I'm kind of waiting on #5359. I think I've figured out most of how I want to do it so once that pr is merged it shouldn't take long |
68d8fb6
to
79080e6
Compare
@jaswilli @novaugust Some of the things I had to do for this seemed to me to be hacks, so I wouold greatly appreciate some feedback on some of the things I did to make this work 😄 |
return `background-image: url(${url})`.htmlSafe(); | ||
}), | ||
|
||
imageBackground: Ember.computed('email', function () { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
02330eb
to
0bf1a19
Compare
* @param {Boolean} hasEmail boolean value corresponding to whether the email property has been passed in | ||
* @param {String} defaultImage String containing the background-image css property of the default user profile image | ||
* @param {String} imageBackground String containing the background-image css property with the gravatar url | ||
*/ |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
0bf1a19
to
058fa41
Compare
Couple of random Travis fails, but I think all the bugs have been ironed out on this one 😄 |
058fa41
to
2df52f5
Compare
@jaswilli & @novaugust any more code review pointers on this one or is it good to go from that perspective? |
Can this be merged before #5399? That way I can kill the merge conflicts there instead of here. Although I guess it doesn't matter, as there will be merge conflicts either way. |
@acburdine I've given this a test run. Only potential problem from a functionality point of view is that if you upload an invalid file, It will not display anything, it should probably be set back to the initial placeholder image in this case |
closes TryGhost#5334 - adds component for profile images (with optional gravatar) - integrates image profile component into setup form
2df52f5
to
b85ac98
Compare
Add profile image upload component
closes #5334
TODOS: