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]: Erratic behavior of paper scroller when adjusting paper #2215

Open
frnora opened this issue Jun 14, 2023 · 5 comments
Open

[Bug]: Erratic behavior of paper scroller when adjusting paper #2215

frnora opened this issue Jun 14, 2023 · 5 comments
Labels
bug jointjs+ The paid commercial extension to the JointJS

Comments

@frnora
Copy link

frnora commented Jun 14, 2023

What happened?

Hi everybody (Roman :) ),

So I am trying to use the paper scroller's autoResizePaper functionality and it "kinda" works but it seems to behave quite erratically. Here is a quick test file (set up for the Test case 3 from below it):

import { dia, shapes, ui } from '@clientio/rappid';

export default function init() {
    let el = document.getElementById('app');
    let paperEl = el.querySelector('.paper');
    let navigatorEl = el.querySelector('.navigator');

    let graph = new dia.Graph();
    let paper = new dia.Paper({
        async: true,
        gridSize: 10,
        drawGrid: {
            name: 'mesh',
            args: {
                color: '#eee'
            }
        },
        width: 2000,
        height: 1000,
        background: {
            color: '#fff'
        },
        sorting: dia.Paper.sorting.APPROX,
        model: graph,
    });
    let paperScroller = new ui.PaperScroller({
        autoResizePaper: true,
        scrollWhileDragging: true,
        baseWidth: 500,
        baseHeight: 200,
        contentOptions: {
            minWidth: 2000,
            minHeight: 1000,
            allowNewOrigin: 'any'
        },
        paper
    });

    paperEl.appendChild(paperScroller.el);
    paperScroller.render();
    
    let nav = new ui.Navigator({
        paperScroller,
        width: 300,
        height: 300,
        zoomOptions: { max: 2, min: 0.5 },
        padding: 20,
        paperOptions: {
            async: true
        }
    });
    
    navigatorEl.appendChild(nav.el);
    nav.render();

    let rectangle = new shapes.standard.Rectangle();
    rectangle.resize(220, 60);
    rectangle.position(50, 100);
    rectangle.attr('label/text', 'Rectangle');
    rectangle.attr('body/fill', 'lightblue');
    rectangle.addTo(graph);

    let circle = new shapes.standard.Circle();
    circle.resize(60, 60);
    circle.position(500, 100);
    circle.attr('label/text', 'Circle');
    circle.attr('body/fill', 'orange');
    circle.addTo(graph);
}

Case 1:
- paper size is 2000 x 2000, no sizes set on the paper scroller

Dragging the circle upwards to the top-edge of the paper moves it close to the center of the paper, below the rectangle.

- changing the paper size to 2000 x 1000
Kinda same result.

There seems to be a jump after the paper scroller adjust the paper which moves the visible area and that causes the element to appear at incorrect x/y.

Case 2:
- paper size is 2000 x 1000, paper scroller baseWidth and baseHeight are 2000 and 1000, so same as the paper
Jump still occurs. Seems like it is there always on first adjust.

Case 3:
- paper size is 2000 x 1000, paper scroller baseWidth: 500, baseHeight: 200, contentOptions: { minWidth: 2000, minHeight: 1000, allowNewOrigin: ‘any’ }

This is probably the worst case of these three. Huge jumps sometimes with big distances. Sometimes moving the element a bit puts it to turbo-speed. Sometimes I “lose” the circle from the navigator. Navigator doesn’t show the correct size.

E.g. these result can be produced imgur (pls excuse the circle label saying 'Rectangle', copy & paste & didn't realize)


Am I doing something wrong? Am I not setting something that should be set or setting something that should be set? I can't see anything very obvious and imho this setup should work.

Is this a bug on the paperscroller?

Version

jointjs+ 3.7

What browsers are you seeing the problem on?

Chrome

What operating system are you seeing the problem on?

Mac

@frnora frnora added the bug label Jun 14, 2023
@kumilingus
Copy link
Contributor

kumilingus commented Jun 16, 2023

The minWidth and minHeight options seem not to be currently supported by the PaperScroller. The problem is that the size of the paper may not change when you drag an element over an edge (it only changes the origin of the paper) and the paper scroller does not know it should update.

I quickly tried to start listening not only to resize events, but also to translate events, and while that helped a bit, it didn't
completely solve the problem.

It depends on what you want to achieve. If you want to define a different initial paper size and increment size, I recommend the following:

contentOptions: () => {
    const minArea = new g.Rect(0, 0, 2000, 1000);
    const contentArea = minArea.union(graph.getBBox());
    return {
        contentArea
    }
}

@frnora
Copy link
Author

frnora commented Jun 29, 2023

I would like to resize the paper while having a base (min size) - so your solution works. Thank you.

However, the initial jump (scroll) still occurs when dragging an item. It happens only when:

  1. the paperScroller is bigger than the available viewport (so it is actually scrollable)
  2. the paperScroller has not been scrolled before dragging the item off the edge

Calling any scroll methods on it, such a .center() or scroll(1000, 1000) then scroll(0, 0) prevents this. Is this intentional behavior or should this 'init' happen automatically but it does not?

EDIT: Actually, I just realized that it still has some issues but I am not sure whether it is related to the navigator or the scroller.
When drag-moving the item 10px up and down (10px is my gridsize), the navigator redraws and shows either a narrower or a wider paper. But in theory the paper size should not be changed at all.
The very same thing happens when I try to adjust the zoom level using the navigator. But here it is even more annoying because it causes frequent flashes. This does not happen when the paper is not resized. Here is what I am talking about: https://imgur.com/a/0iY2QYr

@kumilingus
Copy link
Contributor

Calling any scroll methods on it, such a .center() or scroll(1000, 1000) then scroll(0, 0) prevents this. Is this intentional behavior or should this 'init' happen automatically but it does not?

It should happen automatically but it does not. If I remember correctly, the problem is we don't know when the paper scroller is attached to the DOM (you can set the scrollbars while the container is not in the DOM). It requires a breaking change (for instance throw an error if you try to call render() when the paper scroller is not in the DOM yet).

The very same thing happens when I try to adjust the zoom level using the navigator. But here it is even more annoying because it causes frequent flashes. This does not happen when the paper is not resized. Here is what I am talking about: https://imgur.com/a/0iY2QYr

How to reproduce that? For instance in the Kitchen Sink application?

@frnora
Copy link
Author

frnora commented Jun 30, 2023

It should happen automatically but it does not. If I remember correctly, the problem is we don't know when the paper scroller is attached to the DOM (you can set the scrollbars while the container is not in the DOM). It requires a breaking change (for instance throw an error if you try to call render() when the paper scroller is not in the DOM yet).

That makes sense. Would be nice to point it out in the docs, tho.
Maybe a useless suggestion but couldn't it be fixed by some MutationObserver checking the childList for new entries? (this is just off the top of my head, I have not actually checked the code)

How to reproduce that? For instance in the Kitchen Sink application?

With the code from my first post, just replace the paperScroller with

let paperScroller = new ui.PaperScroller({
    autoResizePaper: true,
    scrollWhileDragging: true,
    baseWidth: 500,
    baseHeight: 500,
    paper,
    contentOptions: () => {
        const minArea = new g.Rect(0, 0, 2000, 1000);
        const contentArea = minArea.union(graph.getBBox());
        return {
            contentArea
        };
    }
});

Then:

  1. Drag one of the elements outside of the initial paper to make it resize
  2. Try to zoom using the navigator

You should be able to see the flashing/size changes of the paper in the navigator.

@kumilingus
Copy link
Contributor

Thank you. I am able to reproduce it now.

The Navigator seems to work as expected. The problem lies within the PaperScroller that resizes the paper in a wrong way after the zoom.

We'll look into it. Please bear with us.

@kumilingus kumilingus added the jointjs+ The paid commercial extension to the JointJS label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug jointjs+ The paid commercial extension to the JointJS
Projects
None yet
Development

No branches or pull requests

2 participants