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

fix(crop-area): recalculate sizes on media objectFit change #506

Merged
merged 1 commit into from
Dec 15, 2023
Merged

fix(crop-area): recalculate sizes on media objectFit change #506

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions src/Cropper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export type CropperProps = {
type State = {
cropSize: Size | null
hasWheelJustStarted: boolean
mediaObjectFit: String | undefined
}

const MIN_ZOOM = 1
Expand Down Expand Up @@ -121,6 +122,7 @@ class Cropper extends React.Component<CropperProps, State> {
state: State = {
cropSize: null,
hasWheelJustStarted: false,
mediaObjectFit: undefined
}

componentDidMount() {
Expand Down Expand Up @@ -218,6 +220,11 @@ class Cropper extends React.Component<CropperProps, State> {
if (prevProps.video !== this.props.video) {
this.videoRef.current?.load()
}

const objectFit = this.getObjectFit();
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to store this in the state? Couldn't just call this.computeSizes when this prop changes (like done for other props above)?

Copy link
Contributor Author

@kruchkou kruchkou Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We could not catch this through props since it's not the props.objectFit changes

What is being changed is the result of getObjectFit() method that calculates how to properly align the image inside the container (should horizontal-cover or vertical-cover be applied)

  • To determine which one should be applied the image should already be presented on the layout to get its sizes (that's why requires re-render)
  • Since we already calculated the sizes and props haven't changed on the second render run, the componentDidUpdate() doesn't call computeSIzes() method

Thats why on getObjectFit() result change we need a callback for computeSizes() and re-render (on state change) to apply new sizes

if (objectFit !== this.state.mediaObjectFit) {
this.setState({mediaObjectFit: objectFit}, this.computeSizes)
}
}

initResizeObserver = () => {
Expand Down Expand Up @@ -350,8 +357,7 @@ class Cropper extends React.Component<CropperProps, State> {
let renderedMediaSize: Size

if (isMediaScaledDown) {
const objectFit = this.getObjectFit()
switch (objectFit) {
switch (this.state.mediaObjectFit) {
default:
case 'contain':
renderedMediaSize =
Expand Down Expand Up @@ -737,7 +743,7 @@ class Cropper extends React.Component<CropperProps, State> {
classes: { containerClassName, cropAreaClassName, mediaClassName },
} = this.props

const objectFit = this.getObjectFit()
const objectFit = this.state.mediaObjectFit;

return (
<div
Expand Down
Loading