Merge branch 'revert-avatar-cropping' into 'master'

Revert the avatar cropping feature

Reverts !2951 since the added 'mini_magick' dependency should be replaced by client-side croppping

See merge request !3223

CHANGELOG

@@ -10,7 +10,6 @@ v 8.6.0 (unreleased)
     setup. A password can be provided during setup (see installation docs), or
     GitLab will ask the user to create a new one upon first visit.
   - Fix issue when pushing to projects ending in .wiki
-  - Fix avatar stretching by providing a cropping feature (Johann Pardanaud)
   - Don't load all of GitLab in mail_room
   - Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set
   - Memoize @group in Admin::GroupsController (Yatish Mehta)

Gemfile

@@ -77,9 +77,6 @@ gem "haml-rails", '~> 0.9.0'
 # Files attachments
 gem "carrierwave", '~> 0.10.0'
 
-# Image editing
-gem "mini_magick", '~> 4.4.0'
-
 # Drag and Drop UI
 gem 'dropzonejs-rails', '~> 0.7.1'
 

Gemfile.lock

@@ -469,7 +469,6 @@ GEM
     method_source (0.8.2)
     mime-types (1.25.1)
     mimemagic (0.3.0)
-    mini_magick (4.4.0)
     mini_portile2 (2.0.0)
     minitest (5.7.0)
     mousetrap-rails (1.4.6)
@@ -957,7 +956,6 @@ DEPENDENCIES
   loofah (~> 2.0.3)
   mail_room (~> 0.6.1)
   method_source (~> 0.8)
-  mini_magick (~> 4.4.0)
   minitest (~> 5.7.0)
   mousetrap-rails (~> 1.4.6)
   mysql2 (~> 0.3.16)

app/assets/javascripts/application.js.coffee

@@ -42,7 +42,6 @@
 #= require jquery.nicescroll
 #= require_tree .
 #= require fuzzaldrin-plus
-#= require cropper.js
 
 window.slugify = (text) ->
   text.replace(/[^-a-zA-Z0-9]+/g, '_').toLowerCase()

app/assets/javascripts/profile.js.coffee

@@ -17,52 +17,14 @@ class @Profile
     $('.update-notifications').on 'ajax:complete', ->
       $(this).find('.btn-save').enable()
 
-    # Avatar management
+    $('.js-choose-user-avatar-button').bind "click", ->
+      form = $(this).closest("form")
+      form.find(".js-user-avatar-input").click()
 
-    $avatarInput = $('.js-user-avatar-input')
-    $filename = $('.js-avatar-filename')
-    $modalCrop = $('.modal-profile-crop')
-    $modalCropImg = $('.modal-profile-crop-image')
-
-    $('.js-choose-user-avatar-button').on "click", ->
-      $form = $(this).closest("form")
-      $form.find(".js-user-avatar-input").click()
-
-    $modalCrop.on 'shown.bs.modal', ->
-      setTimeout ( -> # The cropper must be asynchronously initialized
-        $modalCropImg.cropper
-          aspectRatio: 1
-          modal: false
-          scalable: false
-          rotatable: false
-          zoomable: false
-
-          crop: (event) ->
-            ['x', 'y'].forEach (key) ->
-              $("#user_avatar_crop_#{key}").val(Math.floor(event[key]))
-            $("#user_avatar_crop_size").val(Math.floor(event.width))
-      ), 0
-
-    $modalCrop.on 'hidden.bs.modal', ->
-      $modalCropImg.attr('src', '').cropper('destroy')
-      $avatarInput.val('')
-      $filename.text($filename.data('label'))
-
-    $('.js-upload-user-avatar').on 'click', ->
-      $('.edit-user').submit()
-
-    $avatarInput.on "change", ->
+    $('.js-user-avatar-input').bind "change", ->
       form = $(this).closest("form")
       filename = $(this).val().replace(/^.*[\\\/]/, '')
-      $filename.data('label', $filename.text()).text(filename)
-
-      reader = new FileReader
-
-      reader.onload = (event) ->
-        $modalCrop.modal('show')
-        $modalCropImg.attr('src', event.target.result)
-
-      fileData = reader.readAsDataURL(this.files[0])
+      form.find(".js-avatar-filename").text(filename)
 
 $ ->
   # Extract the SSH Key title from its comment

app/assets/stylesheets/application.scss

@@ -9,7 +9,6 @@
  *= require_self
  *= require dropzone/basic
  *= require cal-heatmap
- *= require cropper.css
 */
 
 /*

app/assets/stylesheets/framework/mixins.scss

@@ -41,12 +41,6 @@
   transition: $transition;
 }
 
-@mixin transform($transform) {
-  -webkit-transform: $transform;
-  -ms-transform: $transform;
-  transform: $transform;
-}
-
 /**
  * Prefilled mixins
  * Mixins with fixed values

app/assets/stylesheets/pages/profile.scss

@@ -109,42 +109,6 @@
   }
 }
 
-.modal-profile-crop {
-  .modal-dialog {
-    width: 500px;
-  }
-
-  .modal-body {
-    p {
-      display: table;
-      margin: auto;
-      overflow: hidden;
-    }
-
-    img {
-      display: block;
-      max-width: 400px;
-      max-height: 400px;
-    }
-
-    .cropper-bg {
-      background: none;
-    }
-
-    .cropper-crop-box {
-      box-sizing: content-box;
-      border: 999px solid transparentize(#ccc, 0.5);
-      @include transform(translate(-999px, -999px));
-    }
-  }
-}
-
-@media (max-width: 520px) {
-  .modal-profile-crop .modal-dialog {
-    width: auto;
-  }
-}
-
 .key-list-item {
   .key-list-item-info {
     @media (min-width: $screen-sm-min) {

app/controllers/profiles_controller.rb

@@ -58,9 +58,6 @@ class ProfilesController < Profiles::ApplicationController
 
   def user_params
     params.require(:user).permit(
-      :avatar_crop_x,
-      :avatar_crop_y,
-      :avatar_crop_size,
       :avatar,
       :bio,
       :email,

app/models/user.rb

@@ -98,9 +98,6 @@ class User < ActiveRecord::Base
   # Virtual attribute for authenticating by either username or email
   attr_accessor :login
 
-  # Virtual attributes to define avatar cropping
-  attr_accessor :avatar_crop_x, :avatar_crop_y, :avatar_crop_size
-
   #
   # Relations
   #
@@ -166,11 +163,6 @@ class User < ActiveRecord::Base
   validate :owns_public_email, if: ->(user) { user.public_email_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
-  validates :avatar_crop_x, :avatar_crop_y, :avatar_crop_size,
-    numericality: { only_integer: true },
-    presence: true,
-    if: ->(user) { user.avatar? && user.avatar_changed? }
-
   before_validation :generate_password, on: :create
   before_validation :restricted_signup_domains, on: :create
   before_validation :sanitize_attrs

app/uploaders/avatar_uploader.rb

@@ -2,22 +2,11 @@
 
 class AvatarUploader < CarrierWave::Uploader::Base
   include UploaderHelper
-  include CarrierWave::MiniMagick
 
   storage :file
 
   after :store, :reset_events_cache
 
-  process :cropper
-
-  def cropper
-    return unless model.respond_to?(:avatar_crop_size) && model.valid?
-
-    manipulate! do |img|
-      img.crop "#{model.avatar_crop_size}x#{model.avatar_crop_size}+#{model.avatar_crop_x}+#{model.avatar_crop_y}"
-    end
-  end
-
   def store_dir
     "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end

app/views/profiles/show.html.haml

@@ -1,7 +1,4 @@
 = form_for @user, url: profile_path, method: :put, html: { multipart: true, class: "edit-user prepend-top-default" }, authenticity_token: true do |f|
-  = f.hidden_field :avatar_crop_x
-  = f.hidden_field :avatar_crop_y
-  = f.hidden_field :avatar_crop_size
   -if @user.errors.any?
     %div.alert.alert-danger
       %ul
@@ -97,19 +94,3 @@
       .prepend-top-default.append-bottom-default
         = f.submit 'Update profile settings', class: "btn btn-success"
         = link_to "Cancel", user_path(current_user), class: "btn btn-cancel"
-
-.modal.modal-profile-crop
-  .modal-dialog
-    .modal-content
-      .modal-header
-        %button.close{type: 'button', data: {dismiss: 'modal'}}
-          %span
-            ×
-        %h4.modal-title
-          Crop your new profile picture
-      .modal-body
-        %p
-          %img.modal-profile-crop-image
-      .modal-footer
-        %button.btn.btn-primary.js-upload-user-avatar{:type => "button"}
-          Set new profile picture

features/steps/profile/profile.rb

@@ -27,7 +27,9 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
   end
 
   step 'I change my avatar' do
-    attach_avatar
+    attach_file(:user_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif'))
+    click_button "Update profile settings"
+    @user.reload
   end
 
   step 'I should see new avatar' do
@@ -40,7 +42,9 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
   end
 
   step 'I have an avatar' do
-    attach_avatar
+    attach_file(:user_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif'))
+    click_button "Update profile settings"
+    @user.reload
   end
 
   step 'I remove my avatar' do
@@ -225,16 +229,4 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
   step "I see that application is removed" do
     expect(page.find(".oauth-applications")).not_to have_content "test_changed"
   end
-
-  def attach_avatar
-    attach_file :user_avatar, Rails.root.join(*%w(spec fixtures banana_sample.gif))
-
-    page.find('#user_avatar_crop_x',    visible: false).set('0')
-    page.find('#user_avatar_crop_y',    visible: false).set('0')
-    page.find('#user_avatar_crop_size', visible: false).set('256')
-
-    click_button "Update profile settings"
-
-    @user.reload
-  end
 end

spec/controllers/namespaces_controller_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe NamespacesController do
-  let!(:user) { create(:user, :with_avatar) }
+  let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
 
   describe "GET show" do
     context "when the namespace belongs to a user" do

spec/controllers/profiles/avatars_controller_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Profiles::AvatarsController do
-  let(:user)    { create(:user, :with_avatar) }
+  let(:user)    { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png")) }
 
   before do
     sign_in(user)

spec/controllers/uploads_controller_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe UploadsController do
-  let!(:user) { create(:user, :with_avatar) }
+  let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
 
   describe "GET show" do
     context "when viewing a user avatar" do

spec/factories/users.rb

@@ -23,13 +23,6 @@ FactoryGirl.define do
       end
     end
 
-    trait :with_avatar do
-      avatar { fixture_file_upload(Rails.root.join(*%w(spec fixtures dk.png)), 'image/png') }
-      avatar_crop_x 0
-      avatar_crop_y 0
-      avatar_crop_size 256
-    end
-
     factory :omniauth_user do
       transient do
         extern_uid '123456'

spec/helpers/application_helper_spec.rb

@@ -77,7 +77,7 @@ describe ApplicationHelper do
     let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
     it 'should return an url for the avatar' do
-      user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(avatar_file_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -88,7 +88,7 @@ describe ApplicationHelper do
       # Must be stubbed after the stub above, and separately
       stub_config_setting(url: Settings.send(:build_gitlab_url))
 
-      user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(avatar_file_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -102,7 +102,7 @@ describe ApplicationHelper do
 
     describe 'using a User' do
       it 'should return an URL for the avatar' do
-        user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+        user = create(:user, avatar: File.open(avatar_file_path))
 
         expect(helper.avatar_icon(user).to_s).
           to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")

spec/models/user_spec.rb

@@ -174,32 +174,6 @@ describe User, models: true do
         end
       end
     end
-
-    describe 'avatar' do
-      it 'only validates when avatar is present and changed' do
-        user = build(:user, :with_avatar)
-
-        user.avatar_crop_x    = nil
-        user.avatar_crop_y    = nil
-        user.avatar_crop_size = nil
-
-        expect(user).not_to be_valid
-        expect(user.errors.keys).
-          to match_array %i(avatar_crop_x avatar_crop_y avatar_crop_size)
-      end
-
-      it 'does not validate when avatar has not changed' do
-        user = create(:user, :with_avatar)
-
-        expect { user.avatar_crop_x = nil }.not_to change(user, :valid?)
-      end
-
-      it 'does not validate when avatar is not present' do
-        user = create(:user)
-
-        expect { user.avatar_crop_y = nil }.not_to change(user, :valid?)
-      end
-    end
   end
 
   describe "Respond to" do

vendor/assets/stylesheets/cropper.css

@@ -1,379 +0,0 @@
-/*!
- * Cropper v2.2.5
- * https://github.com/fengyuanchen/cropper
- *
- * Copyright (c) 2014-2016 Fengyuan Chen and contributors
- * Released under the MIT license
- *
- * Date: 2016-01-18T05:42:29.639Z
- */
-.cropper-container {
-  font-size: 0;
-  line-height: 0;
-
-  position: relative;
-
-  -webkit-user-select: none;
-     -moz-user-select: none;
-      -ms-user-select: none;
-          user-select: none;
-
-  direction: ltr !important;
-  -ms-touch-action: none;
-      touch-action: none;
-  -webkit-tap-highlight-color: transparent;
-  -webkit-touch-callout: none;
-}
-
-.cropper-container img {
-  display: block;
-
-  width: 100%;
-  min-width: 0 !important;
-  max-width: none !important;
-  height: 100%;
-  min-height: 0 !important;
-  max-height: none !important;
-
-  image-orientation: 0deg !important;
-}
-
-.cropper-wrap-box,
-.cropper-canvas,
-.cropper-drag-box,
-.cropper-crop-box,
-.cropper-modal {
-  position: absolute;
-  top: 0;
-  right: 0;
-  bottom: 0;
-  left: 0;
-}
-
-.cropper-wrap-box {
-  overflow: hidden;
-}
-
-.cropper-drag-box {
-  opacity: 0;
-  background-color: #fff;
-
-  filter: alpha(opacity=0);
-}
-
-.cropper-modal {
-  opacity: .5;
-  background-color: #000;
-
-  filter: alpha(opacity=50);
-}
-
-.cropper-view-box {
-  display: block;
-  overflow: hidden;
-
-  width: 100%;
-  height: 100%;
-
-  outline: 1px solid #39f;
-  outline-color: rgba(51, 153, 255, .75);
-}
-
-.cropper-dashed {
-  position: absolute;
-
-  display: block;
-
-  opacity: .5;
-  border: 0 dashed #eee;
-
-  filter: alpha(opacity=50);
-}
-
-.cropper-dashed.dashed-h {
-  top: 33.33333%;
-  left: 0;
-
-  width: 100%;
-  height: 33.33333%;
-
-  border-top-width: 1px;
-  border-bottom-width: 1px;
-}
-
-.cropper-dashed.dashed-v {
-  top: 0;
-  left: 33.33333%;
-
-  width: 33.33333%;
-  height: 100%;
-
-  border-right-width: 1px;
-  border-left-width: 1px;
-}
-
-.cropper-center {
-  position: absolute;
-  top: 50%;
-  left: 50%;
-
-  display: block;
-
-  width: 0;
-  height: 0;
-
-  opacity: .75;
-
-  filter: alpha(opacity=75);
-}
-
-.cropper-center:before,
-.cropper-center:after {
-  position: absolute;
-
-  display: block;
-
-  content: ' ';
-
-  background-color: #eee;
-}
-
-.cropper-center:before {
-  top: 0;
-  left: -3px;
-
-  width: 7px;
-  height: 1px;
-}
-
-.cropper-center:after {
-  top: -3px;
-  left: 0;
-
-  width: 1px;
-  height: 7px;
-}
-
-.cropper-face,
-.cropper-line,
-.cropper-point {
-  position: absolute;
-
-  display: block;
-
-  width: 100%;
-  height: 100%;
-
-  opacity: .1;
-
-  filter: alpha(opacity=10);
-}
-
-.cropper-face {
-  top: 0;
-  left: 0;
-
-  background-color: #fff;
-}
-
-.cropper-line {
-  background-color: #39f;
-}
-
-.cropper-line.line-e {
-  top: 0;
-  right: -3px;
-
-  width: 5px;
-
-  cursor: e-resize;
-}
-
-.cropper-line.line-n {
-  top: -3px;
-  left: 0;
-
-  height: 5px;
-
-  cursor: n-resize;
-}
-
-.cropper-line.line-w {
-  top: 0;
-  left: -3px;
-
-  width: 5px;
-
-  cursor: w-resize;
-}
-
-.cropper-line.line-s {
-  bottom: -3px;
-  left: 0;
-
-  height: 5px;
-
-  cursor: s-resize;
-}
-
-.cropper-point {
-  width: 5px;
-  height: 5px;
-
-  opacity: .75;
-  background-color: #39f;
-
-  filter: alpha(opacity=75);
-}
-
-.cropper-point.point-e {
-  top: 50%;
-  right: -3px;
-
-  margin-top: -3px;
-
-  cursor: e-resize;
-}
-
-.cropper-point.point-n {
-  top: -3px;
-  left: 50%;
-
-  margin-left: -3px;
-
-  cursor: n-resize;
-}
-
-.cropper-point.point-w {
-  top: 50%;
-  left: -3px;
-
-  margin-top: -3px;
-
-  cursor: w-resize;
-}
-
-.cropper-point.point-s {
-  bottom: -3px;
-  left: 50%;
-
-  margin-left: -3px;
-
-  cursor: s-resize;
-}
-
-.cropper-point.point-ne {
-  top: -3px;
-  right: -3px;
-
-  cursor: ne-resize;
-}
-
-.cropper-point.point-nw {
-  top: -3px;
-  left: -3px;
-
-  cursor: nw-resize;
-}
-
-.cropper-point.point-sw {
-  bottom: -3px;
-  left: -3px;
-
-  cursor: sw-resize;
-}
-
-.cropper-point.point-se {
-  right: -3px;
-  bottom: -3px;
-
-  width: 20px;
-  height: 20px;
-
-  cursor: se-resize;
-
-  opacity: 1;
-
-  filter: alpha(opacity=100);
-}
-
-.cropper-point.point-se:before {
-  position: absolute;
-  right: -50%;
-  bottom: -50%;
-
-  display: block;
-
-  width: 200%;
-  height: 200%;
-
-  content: ' ';
-
-  opacity: 0;
-  background-color: #39f;
-
-  filter: alpha(opacity=0);
-}
-
-@media (min-width: 768px) {
-  .cropper-point.point-se {
-    width: 15px;
-    height: 15px;
-  }
-}
-
-@media (min-width: 992px) {
-  .cropper-point.point-se {
-    width: 10px;
-    height: 10px;
-  }
-}
-
-@media (min-width: 1200px) {
-  .cropper-point.point-se {
-    width: 5px;
-    height: 5px;
-
-    opacity: .75;
-
-    filter: alpha(opacity=75);
-  }
-}
-
-.cropper-invisible {
-  opacity: 0;
-
-  filter: alpha(opacity=0);
-}
-
-.cropper-bg {
-  background-image: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQAQMAAAAlPW0iAAAAA3NCSVQICAjb4U/gAAAABlBMVEXMzMz////TjRV2AAAACXBIWXMAAArrAAAK6wGCiw1aAAAAHHRFWHRTb2Z0d2FyZQBBZG9iZSBGaXJld29ya3MgQ1M26LyyjAAAABFJREFUCJlj+M/AgBVhF/0PAH6/D/HkDxOGAAAAAElFTkSuQmCC');
-}
-
-.cropper-hide {
-  position: absolute;
-
-  display: block;
-
-  width: 0;
-  height: 0;
-}
-
-.cropper-hidden {
-  display: none !important;
-}
-
-.cropper-move {
-  cursor: move;
-}
-
-.cropper-crop {
-  cursor: crosshair;
-}
-
-.cropper-disabled .cropper-drag-box,
-.cropper-disabled .cropper-face,
-.cropper-disabled .cropper-line,
-.cropper-disabled .cropper-point {
-  cursor: not-allowed;
-}