fix: Fixed auto align to bounds bugs

[nuintun]

Fixed auto align to bounds bugs: #485 (comment)

[prc5]

Looks great, few adjustments and we can ship it!
Also please fix the typescript issues from the previous PR so it will be bundling again - and this way it will be possible to release it.

[prc5]

I think this is too heavy for the animation frame and too hacky.

[nuintun]

There is some impact from the animation frame, but because of cancelAnimationFrame, the impact is limited.

[prc5]

I would add the customTime parameter to handleAlignToBounds so it would be:

handleAlignToBounds(this, 0)

So the alignment will be instant - or controlled via new prop resizeAlignmentTime: number

[nuintun]

This is a very good implementation👍.

[prc5]

Can you add it here too?

[nuintun]

OK，I will add this, but I think that normal mouse pinning should not delay alignment bounds; only resizing needs to.

handleAlignToBounds(this, 0) is not necessary.

Just needs:

clearTimeout(timerId);

timerId = setTimeout(() => {
  handleCancelAnimation(this);
  handleCalculateBounds(this, scale);
  handleAlignToBounds(this);
}, resizeAlignmentTime);

[prc5]

There is no need for it to limit the mouse panning. Check this out:

export function handleAlignToBounds(
  contextInstance: ReactZoomPanPinchContext,
  // one addition here! it is still optional
  customAlignmentTime?: number
): void {
  const { scale } = contextInstance.state;
  const { minScale, autoAlignment } = contextInstance.setup;
  const { disabled, sizeX, sizeY, animationTime, animationType } =
    autoAlignment;

  const isDisabled = disabled || scale < minScale || (!sizeX && !sizeY);

  if (isDisabled) return;

  const targetState = handlePanToBounds(contextInstance);

  if (targetState) {
    animate(
       contextInstance, 
       targetState, 
       // and here! we only apply it for cases where customAlignment is needed
       customAlignmentTime ?? animationTime, 
       animationType
    );
  }
}

This way there is no need for the setTimeout 😄

[prc5]

Btw. Thank you for your work, that's a great contribution! 🙌🏻

[nuintun]

@prc5 Typescript issues fixed, and also fixed build tools warnings.

[prc5]

I think this is not a good solution for this case since it's not representing true state. We initialize observer with undefined value so in this case it is giving us the falsy assumption that it is always there by ! casting. You should adjust logic to handle observer being undefined value instead.

Like it was done here: https://github.com/BetterTyped/react-zoom-pan-pinch/pull/485/files#diff-e358387c9a984c08ecacfd998a21a99ffbe7156e0eca96588cedce311d5a65acL160

[nuintun]

Because the observer already exists after initialization.
The code here does not have conditional branches like before: https://github.com/nuintun/react-zoom-pan-pinch/blob/0a2e348e1847e6bb31283104fd0b88fe1fdea796/src/core/instance.core.ts#L199

[prc5]

The problem is that initialization is a method, not a constructor so it could be triggered at different times - for example, just after rendering.
So the lifecycle is like:

1. We make instance new ReactZoomPanPinch()
2. Reacts lifecycle kicks in and renders the content for x time
3. Then we trigger initialize on useLayoutEffect - this is the time when wrapper and content is mounted to DOM

Between 1 and 3 there is some processing on the React's side which is taking unspecified amount of time based on the component you're building. During this time observer is undefined so this implementation is not truthy.

So my approach would be to keep it as close as possible to the real flow. Typescript must reflect the real world otherwise it is becoming a problem to handle later on.

[nuintun]

OK, I will fixed this.

[prc5]

I'm still not convinced why we need the requestAnimationFrame instead of just calling the alignToBounds once. I cannot accept this as it may cause some uncontrolled memory leaks. Let's try to do it without it.

[nuintun]

I'm still not convinced why we need the requestAnimationFrame instead of just calling the alignToBounds once. I cannot accept this as it may cause some uncontrolled memory leaks. Let's try to do it without it.

After having resizeAlignmentTime, requestAnimationFrame can be removed😁.

Using requestAnimationFrame here is just because I don’t want handleAlignToBounds to be triggered frequently, but I also want to make handleAlignToBounds trigger as soon as possible.

[prc5]

Amazing work, thank you! 🚀

[github-actions]

🎉 This PR is included in version 3.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀