Clebeuf / beLocal

Connecting local farmers and foodmakers with the community.
http://belocalvictoria.me
Other
4 stars 0 forks source link

Image crop #95

Closed rpanjwani closed 9 years ago

rpanjwani commented 9 years ago

sleeeeeep

rpanjwani commented 9 years ago

@scottlow I'm getting merge conflicts and I'm wondering if the following portion can all be deleted as we probably don't need it. Are you doing anything aside from clearing the profile-image here?

 // Called to reset edit profile modal
 $scope.editProfile = function() {
 <<<<<<< HEAD
    var e = angular.element('#profile-image');
    e.wrap('<form>').closest('form').get(0).reset();
    e.unwrap();
  =======
    $scope.profileImageError = undefined; // Reset error message on photo

    // Strange hack to clear an image uploader. It turns out that wrapping it in a <form> and resetting that form before unwrapping it works like a charm.
    var e = angular.element('#profile-image');
    e.wrap('<form>').closest('form').get(0).reset();
    e.unwrap();

    $scope.displayProfileThumbnail = $scope.currentUser.vendor.photo ? true : false; // Should we display a thumbail image preview for profile photo?

    if($scope.displayProfileThumbnail)
        angular.element('#profilePreview').attr('src', $scope.currentUser.vendor.photo.image_url).width(50).height(50); // If so, set the profile image thumbnail

master }

scottlow commented 9 years ago

Yeah go ahead and delete that. That was old code for clearing the image thumbnail/picker that shouldn't be necessary with your image crop changes.

scottlow commented 9 years ago

@rpanjwani @Clebeuf before this goes in, we should probably remove the resizeStep() function from seller.js since I don't believe we're going to use it anymore with the new workflow.

scottlow commented 9 years ago

@rpanjwani Also, right now, imagecropped.js is an empty JavaScript file. Can we move the directive from seller.js into there?

scottlow commented 9 years ago

@rpanjwani

In views.py we have this line: vendor = Vendor.objects.get(pk = request.DATA["vendorId"])

This is insecure, as any vendor id could be passed into the request. A better way of doing this is to do something like

vendor = Vendor.objects.get(user=request.user) as this ensures that 1) the user has authenticated with our back end and 2) only their account can be modified by them.

Do you mind making this change? And removing the vendor_id passed in from the client side as well?

rpanjwani commented 9 years ago

@scottlow thanks scott, I've made the recommended changes. The only thing remains now is to test the vendor profile hover transition on IE. Maybe we can have @CakeBrewery test it today during the meeting.

scottlow commented 9 years ago

@rpanjwani Looks good, except I think there's an extra f now on line 268 of seller.js :P

Clebeuf commented 9 years ago

@rpanjwani @scottlow don't merge this in until I make the changes to the way that we display profile images. I'm going to need a bit of time to fiddle with the responsiveness and add a bunch of media queries for different breakpoints. Ill try to get to it this afternoon / this evening :)

rpanjwani commented 9 years ago

thanks @scottlow for noticing. It has been updated.

Clebeuf commented 9 years ago

I fixed the way that we display images for the vendor details and the vendor (personal) pages. This means that we need the vendors to have images of size 500x300 which is being forced by the crop.

Clebeuf commented 9 years ago

I also added bootstrap tour for the new cropper. If you're testing this out don't forget that bower sometimes adds in the standalone version on top of the min version of bootstrap tour in index.html. This causes the pointer thing so misalign, but removing the standalone css & js solves that.

Clebeuf commented 9 years ago

:+1: Im okay with this pull request going in, but it's probs better to do it at night since we don't know how well it will work on mobile

scottlow commented 9 years ago

This looks good to me! I'm gonna pull it in since it's been signed off by @rpanjwani and @Clebeuf and make sure everything works on deployment.

rpanjwani commented 9 years ago

@Clebeuf @scottlow just tested img-cropper live quickly and noticed the image overlay doesn't happen on hover. Also this file is not found: https://belocalvictoria.me/media/vendors/Cover.png . Maybe they are related?

EDIT: Sorry, I just refreshed the profile page and now the hover overlay works?! The console is still complaining about the above file missing tho :)

Clebeuf commented 9 years ago

It works perfectly for me on chrome, firefox, and safari

scottlow commented 9 years ago

@rpanjwani @Clebeuf What is Cover.png? An image you tried to upload? Or something the image-cropper needs? You might be running into a weird caching issue or something. Let us know if you have more concrete steps to reproduce!