Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 9 years ago

#4859 closed enhancement

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

Reported by: modemlooper's profile 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
12 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
12 years ago

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

#3 @DJPaul
12 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
12 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
12 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
12 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
12 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
11 years ago

  • Milestone changed from 1.8 to Future Release

#9 @terraling
11 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
11 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 11 years ago by modemlooper (previous) (diff)

#11 @boonebgorges
11 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
11 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
10 years ago

  • Cc mercijavier@… added

#14 @DJPaul
9 years ago

  • Keywords has-patch needs-testing removed

@imath Can we close this now?

#15 @imath
9 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
9 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.