Skip to content
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.

Sign up for GitHub

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

feat: planter selfie rotation #142

Merged
merged 6 commits into from
Sep 15, 2021
Merged

feat: planter selfie rotation #142

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a636045
feat: planter selfie rotation
tranquanghuy0801 Aug 4, 2021
1863c6b
fix: re-format the apply rotation planter image
tranquanghuy0801 Aug 9, 2021
624f386
fix: fix the overwrite on-click rotation
tranquanghuy0801 Aug 10, 2021
d2de60e
fix: fix the overflow image rotation
tranquanghuy0801 Aug 15, 2021
6c2305f
fix: update the rotation for multiple images
tranquanghuy0801 Aug 23, 2021
ee65107
fix: make all planter images square for ease of rotation
nmcharlton Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/api/planters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default {
personId: true,
organization: true,
organizationId: true,
imageRotation: true,
id: true,
},
};
Expand Down
9 changes: 4 additions & 5 deletions src/components/EditPlanter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,6 @@ const EditPlanter = (props) => {
}
}

function handleSelectPlanterImage(img) {
handleChange('imageUrl', img);
}

function getValue(attr) {
// Ensure empty strings are not overlooked
if (planterUpdate?.[attr] != null) {
Expand Down Expand Up @@ -146,9 +142,12 @@ const EditPlanter = (props) => {
<ImageScroller
images={planterImages}
selectedImage={planterUpdate?.imageUrl || planter.imageUrl}
onSelectImage={handleSelectPlanterImage}
loading={loadingPlanterImages}
blankMessage="No planter images available"
imageRotation={
planterUpdate?.imageRotation || planter.imageRotation || 0
}
onSelectChange={handleChange}
/>
{inputs.map((row, rowIdx) => (
<Grid item container direction="row" key={rowIdx}>
Expand Down
51 changes: 47 additions & 4 deletions src/components/ImageScroller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useState, useRef } from 'react';
import { makeStyles } from '@material-ui/core/styles';
import { Fab, Grid, CircularProgress, Card, Button } from '@material-ui/core';
import { ChevronLeft, ChevronRight } from '@material-ui/icons';
import Rotate90DegreesCcwIcon from '@material-ui/icons/Rotate90DegreesCcw';
import OptimizedImage from './OptimizedImage';

/* This component currently uses fixed size cards and scroll window,
Expand Down Expand Up @@ -29,7 +30,7 @@ const useStyle = makeStyles((theme) => ({
textAlign: 'center',
},
imageCard: {
height: '100%',
height: '84%',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it simple and make these images square.

margin: theme.spacing(1.5),
width: `${IMAGE_CARD_SIZE - theme.spacing(3)}px`,
cursor: 'pointer',
Expand Down Expand Up @@ -65,20 +66,38 @@ const useStyle = makeStyles((theme) => ({
textAlign: 'center',
height: '100%',
},
clickRotate: {
margin: 5,
position: 'absolute',
bottom: 0,
right: 0,
width: 32,
height: 32,
},
}));

export default function ImageScroller(props) {
const {
images,
selectedImage,
onSelectImage,
loading = false,
blankMessage = '',
imageRotation,
onSelectChange,
} = props;

const classes = useStyle();
const [maxImages, setMaxImages] = useState(MAX_IMAGES_INCREMENT);
const imageScrollerRef = useRef(null);
let objRotation = {};
if (selectedImage) {
const selectedIndex = images.indexOf(selectedImage);
objRotation[selectedIndex] = imageRotation;
} else {
images.map((_, idx) => {
objRotation[idx] = imageRotation;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really make sense to apply image rotation to all images if none is selected.
Just leave all other images unrotated and apply the rotation to the selected image only.

});
}
let [rotations, setRotations] = useState(objRotation);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is introducing unnecessary complexity.
There's no point in persisting the image rotation for other images in the frontend like this; only one value gets stored for each planter so all other rotations will be lost when they refresh (which is annoying for a user).


function loadMoreImages() {
setMaxImages(maxImages + MAX_IMAGES_INCREMENT);
Expand All @@ -103,6 +122,16 @@ export default function ImageScroller(props) {
});
}

function handleRotationChange(idx) {
let newRotation =
rotations[idx] >= 0 ? rotations[idx] + 90 : imageRotation + 90;
if (newRotation === 360) {
newRotation = 0;
}
setRotations({ ...rotations, [idx]: newRotation });
onSelectChange('imageRotation', newRotation);
}

return (
<div className={classes.container}>
<Grid
Expand All @@ -118,7 +147,6 @@ export default function ImageScroller(props) {
images.slice(0, maxImages).map((img, idx) => (
<Card
key={`${idx}_${img}`}
onClick={() => onSelectImage(img)}
className={`image-card ${classes.imageCard} ${
img === selectedImage && classes.selectedImageCard
}`}
Expand All @@ -129,7 +157,22 @@ export default function ImageScroller(props) {
height={192}
className={classes.image}
fixed
rotation={rotations[idx] ? rotations[idx] : imageRotation}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest reverting back to the single value, but only applying it if this is the selected image.
When the user selects a new image, just reset the rotation to zero.

onClick={() => onSelectChange('imageUrl', img)}
/>
{img === selectedImage ? (
<Fab
id="click-rotate"
className={classes.clickRotate}
onClick={() => handleRotationChange(idx)}
>
<Rotate90DegreesCcwIcon
style={{ transform: `rotateY(180deg)` }}
/>
</Fab>
) : (
''
)}
</Card>
))
) : (
Expand Down
2 changes: 2 additions & 0 deletions src/components/OptimizedImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default function OptimizedImage(props) {
screenWidths = [1600, 1280, 960, 0],
imageSizes = [400, 300, 250, 200],
fixed,
rotation,
...rest
} = props;

Expand Down Expand Up @@ -53,6 +54,7 @@ export default function OptimizedImage(props) {
objectFit: 'cover',
width: '100%',
height: '100%',
transform: `rotate(${rotation}deg)`,
}}
{...rest}
/>
Expand Down
13 changes: 9 additions & 4 deletions src/components/PlanterDetail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ import OptimizedImage from './OptimizedImage';
import LinkToWebmap from './common/LinkToWebmap';

const useStyle = makeStyles((theme) => ({
root: {
width: 441,
},
box: {
padding: theme.spacing(4),
},
Expand Down Expand Up @@ -108,7 +105,14 @@ const PlanterDetail = (props) => {
return (
<React.Fragment>
<Drawer anchor="right" open={props.open} onClose={props.onClose}>
<Grid className={classes.root}>
<Grid
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make the container and image square to avoid this awkwardness and inconsistency between rotated and unrotated images.

The planter card can also be updated to use a square image.

style={{
width:
planter.imageRotation === 0 || planter.imageRotation === 180
? 441
: 378,
}}
>
<Grid container direction="column">
<Grid item>
<Grid container justify="space-between" alignItems="center">
Expand All @@ -134,6 +138,7 @@ const PlanterDetail = (props) => {
height={378}
className={classes.cardMedia}
fixed
rotation={planter.imageRotation}
/>
)}
{!planter.imageUrl && (
Expand Down
1 change: 1 addition & 0 deletions src/components/Planters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ function Planter(props) {
height={192}
className={classes.cardMedia}
fixed
rotation={planter.imageRotation}
/>
)}
{!planter.imageUrl && (
Expand Down