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.

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

[Bug]: viewportScreenBounds calculated wrong after load a snapshot from different screen size #3705

Open
1 task done
merobal opened this issue May 6, 2024 · 4 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@merobal
Copy link

merobal commented May 6, 2024

What happened?

We have a content editor inside our page (a worksheet for students). We call editor.zoomToFit() after saving the image / clicking on cancel, and it works (almost) well with v2.0, but it's totally broken with v2.1

How can we reproduce the bug?

I posted a video about it on Discord

What browsers are you seeing the problem on?

Chrome

Contact Details

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@merobal merobal added the bug Something isn't working label May 6, 2024
Copy link

linear bot commented May 6, 2024

@merobal
Copy link
Author

merobal commented May 7, 2024

https://stackblitz.com/edit/vitejs-vite-jnqyji?file=src%2FApp.tsx

  1. Edit
  2. Draw something
  3. Save
  4. Resize the screen
  5. Edit again
  6. Cancel (loads back the saved snapshot from different viewport size)
  7. Edit again => broken

@merobal merobal changed the title [Bug]: editor.zoomToFit is broken with v2.1 [Bug]: viewportScreenBounds calculated wrong after load a snapshot from different screen size May 7, 2024
@ds300
Copy link
Collaborator

ds300 commented May 21, 2024

Hi! Sorry for the delay in getting back to you.

This issue is caused by using the 'all' option when calling editor.store.getSnapshot('all'). This saves literally all of the information in the in-memory store, which includes session-specific stuff like the viewport size, or which shape is currently being hovered over, and so on. For lots of this data it doesn't make sense to serialize and reinstate it blindly.

If you need to persist certain bits of UI state (camera position, current page, which shapes are selected, etc) then we have some separate tools for persisting and reloading this kind of 'session' state (look for the substring sessionState in this file), but they're currently not documented well and the api is not as user-friendly as it could be. I'm going to change that because you're not the first person to run into this kind of issue.

In the meantime it looks like the app you've linked doesn't need that so you can probably just remove the 'all' param.

@merobal
Copy link
Author

merobal commented May 21, 2024

Hi! Sorry, I just posted a very few pieces of info here, but we discussed this in detail on Discord and found the workaround, too!

So even call getSnapshot without the param all the same bug happens.

But calling this snippet after loadSnapshot fixes the issue:

    const rect = container.current.getBoundingClientRect();
    const next = new Box(
      rect.left || rect.x,
      rect.top || rect.y,
      Math.max(rect.width, 1),
      Math.max(rect.height, 1)
    );
    editor.updateViewportScreenBounds(next);

@MitjaBezensek led me to find this solution.

The actual fix could be a higher-level editor.load functionality or just mentioning this snippet in the snapshots docs (or just creating a dedicated public API for this snippet to call as a single line).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants