Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 4 years ago

#4859 closed enhancement

Avatar Cropper Small Screens - Image Size Too Large on mobile devices

Reported by: modemlooper Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.2
Component: Core Keywords:
Cc: mercijavier@…

Description

Need the avatar image resized on mobile so it fits into the screen. I came up with this if anyone wants to try it or come up with something better. I just added a mobile detect and then resize image to 280. 280 should fit into most mobile screens when in portrait.

function bp_core_add_cropper_inline_js() {

	if (  wp_is_mobile()  ) {

		$imageraw = wp_get_image_editor(  bp_core_avatar_upload_path() . buddypress()->avatar_admin->image->dir  );

		if (  ! is_wp_error( $imageraw  )  ) {
		    $imageraw->resize(  280, 300, false );
		    $imageraw->save(  bp_core_avatar_upload_path() . buddypress()->avatar_admin->image->di r );
	    }
    }

	// Bail if no image was uploaded
	$image = apply_filters( 'bp_inline_cropper_image', getimagesize( bp_core_avatar_upload_path() . buddypress()->avatar_admin->image->dir ) );
	if ( empty( $image ) )
		return;

	//
	$full_height = bp_core_avatar_full_height();
	$full_width  = bp_core_avatar_full_width();

	// Calculate Aspect Ratio
	if ( !empty( $full_height ) && ( $full_width != $full_height ) ) {
		$aspect_ratio = $full_width / $full_height;
	} else {
		$aspect_ratio = 1;
	}

	// Default cropper coordinates
	$crop_left   = $image[0] / 4;
	$crop_top    = $image[1] / 4;
	$crop_right  = $image[0] - $crop_left;
	$crop_bottom = $image[1] - $crop_top; ?>

	<script type="text/javascript">
		jQuery(window).load( function(){
			jQuery('#avatar-to-crop').Jcrop({
				onChange: showPreview,
				onSelect: showPreview,
				onSelect: updateCoords,
				aspectRatio: <?php echo $aspect_ratio; ?>,
				setSelect: [ <?php echo $crop_left; ?>, <?php echo $crop_top; ?>, <?php echo $crop_right; ?>, <?php echo $crop_bottom; ?> ]
			});
			updateCoords({x: <?php echo $crop_left; ?>, y: <?php echo $crop_top; ?>, w: <?php echo $crop_right; ?>, h: <?php echo $crop_bottom; ?>});
		});

		function updateCoords(c) {
			jQuery('#x').val(c.x);
			jQuery('#y').val(c.y);
			jQuery('#w').val(c.w);
			jQuery('#h').val(c.h);
		}

		function showPreview(coords) {
			if ( parseInt(coords.w) > 0 ) {
				var fw = <?php echo $full_width; ?>;
				var fh = <?php echo $full_height; ?>;
				var rx = fw / coords.w;
				var ry = fh / coords.h;

				jQuery( '#avatar-crop-preview' ).css({
					width: Math.round(rx * <?php echo $image[0]; ?>) + 'px',
					height: Math.round(ry * <?php echo $image[1]; ?>) + 'px',
					marginLeft: '-' + Math.round(rx * coords.x) + 'px',
					marginTop: '-' + Math.round(ry * coords.y) + 'px'
				});
			}
		}
	</script>

<?php
}

Change History (16)

#1 @boonebgorges
7 years ago

  • Keywords has-patch needs-testing added

This logic may need to be reworked a little bit, but the approach seems fine to me.

#2 @modemlooper
7 years ago

absolutely, it was just a quick "can it work with minimal changes to current code"

#3 @DJPaul
7 years ago

I'm more keen to redesign the crop avatar page to make this better on mobile. Unless there's something particular going on here with the javascript, both the "uploaded avatar" and the "cropped image" boxes could be floated.

#4 @modemlooper
7 years ago

Paul, when you upload the image displays full size so floating alone won't be enough. I'm going to play more with this. I think it would be better to not have the drag cropper on mobile and use pinching to zoom the image inside a box. The drag cropper doesn't work 100%. Once you drag and release it's very difficult to resize the cropper.

#5 @DJPaul
7 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Avatar Cropper Small Screens - Image Size Too Large to Avatar Cropper Small Screens - Image Size Too Large on mobile devices
  • Version changed from 1.7 to 1.2

#6 @modemlooper
7 years ago

found a fix for this from jcrop forums. There is an arg you pass in the array to read the true size not the displayed size

trueSize: [<?echo $full_width;?>,<?echo $full_height;?>]

#7 @DJPaul
7 years ago

  • Milestone changed from Future Release to 1.8

Hey, modemlooper -- up for getting this into 1.8? Not sure if the fix needs a refresh, but it'd be good to get a proper patch file made.

#8 @boonebgorges
6 years ago

  • Milestone changed from 1.8 to Future Release

#9 @terraling
6 years ago

I see there are a few tickets around improving avatar uploading and cropping, and I'm commenting on this one because it's specifically about the experience on mobile.

A few observations:

  1. jcrop "works" on mobile but I find is barely usable, the handles are too small etc., and a pinch-zoom approach would be better.
  2. no rotate -- my users are using their phones to take a snapshot for their avatar which when uploaded is often sideways-on or upside-down with no means to fix other than start-over and use a photo-app first.
  3. elsewhere (https://buddypress.trac.wordpress.org/ticket/5146) it has been suggested 'streamlining' the upload by removing the requirement to press an upload button and have it be triggered on changing the file select field and doing the cropping on the server etc.

This is the opposite of what should be happening. Many more users access sites with mobile (duh) and a modern approach that caters for them should look something like:

-- User selects a photo to upload. If on mobile this may well be a 5Mb image for a 150px avatar. So,
-- User can crop AND rotate photo as required
-- This is done IN THE CLIENT using canvas so that the image can be resized BEFORE uploading (esp. via a mobile data plan). Less bandwidth, & less server storage.
-- As a fallback for non canvas-capable browsers and for re-editing existing images (crop/rotate) options should be available via the server.

There is quite a lot of work to be done here and I have to do it for my own site, and I'm currently investigating alternatives to jcrop etc., and I just wanted to put a marker down to say that I'm working on it and will let you know how I get on.

I already have the front-end image cropping/rotating canvas stuff working on desktop (for uploading images in front-end forms for another area of my site, haven't changed the avatar uploads yet), though the library I used for cropping is even worse than jcrop on mobile.

I'll keep you posted.

#10 @modemlooper
6 years ago

I doubt a new method for cropping will be implemented for mobile. My patch works for fixing rotate issue. BuddyPress uses jcropper included in core. You can however create a plugin to use an alternative method. This IMHO is theme related as its a UI/UX issue.

https://buddypress.trac.wordpress.org/ticket/5089

responsive fix for jcrop http://stackoverflow.com/questions/13648162/using-jcrop-on-responsive-images

Last edited 6 years ago by modemlooper (previous) (diff)

#11 @boonebgorges
6 years ago

Thanks, terraling. Looking forward to seeing what you come up with.

This IMHO is theme related as its a UI/UX issue.

I would agree, except that BP itself currently handles all UX related to avatar upload and cropping. If we're going to provide this service, we should do it correctly.

#12 @modemlooper
6 years ago

jcrop works fine on mobile if it is coded properly for responsive. Just don't see a need to redo the whole thing with something non core.

If this were an app, then yes a canvas based photo edit would be better.

#13 @mercime
5 years ago

  • Cc mercijavier@… added

#14 @DJPaul
4 years ago

  • Keywords has-patch needs-testing removed

@imath Can we close this now?

#15 @imath
4 years ago

@djpaul i'm not sure. It looks like there's a trouble in the new avatar UI #6586 and here we're talking about the old UI and as we support back to 3.8: this problem can appear in these configs or if the admin disabled the new UI.

#16 @imath
4 years ago

  • Milestone Future Release deleted
  • Status changed from new to closed

Ok i've added a patch on #6586 which is talking about the same issue from my point of view. So closing this ticket in favor of #6586.

Note: See TracTickets for help on using tickets.