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

Checking restrict graph to viewport makes the D-pad and zoom slider on the viewport disappear #1083

Closed
kdahlquist opened this issue Jan 23, 2024 · 12 comments

Comments

@kdahlquist
Copy link
Collaborator

Beta 7.0.8

  • Checking restrict graph to viewport makes the D-pad and zoom slider on the viewport disappear. It seems to reappear if unchecked again.
@ceciliazaragoza
Copy link
Collaborator

ceciliazaragoza commented Feb 3, 2024

@kdahlquist I was able to remove the D-pad disappearing issue. Also, in the code it seems like the disappearing was expected behavior since the user should be able to view all the nodes in the graph without moving around the graph. Should I also make it so that the user can use the D-pad and zoom slider to navigate around the graph and allow the user to use their cursor to navigate around the graph?

@kdahlquist
Copy link
Collaborator Author

@ceciliazaragoza, I'm not sure what you mean by "the user can use the D-pad and zoom slider to navigate around the graph and allow the user to use their cursor to navigate around the graph". Currently, you can "grab" the whole graph and move it around with a mouse click and drag. I think the issue is that sometimes the D-pad is there and sometimes it isn't. Maybe we need a check box somewhere to enable/disable the D-pad/zoom sliders, so it is under user control and it's clear when it is enabled or not.

@kdahlquist
Copy link
Collaborator Author

Oh, I see that when "Restrict graph to viewport" is checked, you can no longer move the graph with your mouse. We should still be able to move the graph around. We should be able to move it as long as it doesn't go outside the viewport.

ceciliazaragoza added a commit that referenced this issue Feb 5, 2024
…want the user to be able to use the D-pad and zoom slider or disable them
ceciliazaragoza added a commit that referenced this issue Feb 6, 2024
…E, and the zoom slider can only be moved to 100% as well
ceciliazaragoza added a commit that referenced this issue Feb 6, 2024
… border of graph though. Need to make sure that can only move cursor to area in graph as well
ceciliazaragoza added a commit that referenced this issue Feb 6, 2024
…om value changed and in restrict graph to viewpoint so that do not exceed viewport borders. Also cleaned up code in zoomDragged to be cleaner
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…me comments and moved the instantiation of CURSOR_CLASSES to an earlier line
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…ds(width, height) tracks if the graph is in bounds, and updateZoomContainerInfo is called from inside that function that updates variables that store zoomContainerInfo so that code easier to read. Issue #1083.
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…his to see if cursorGrab is on or not, but for some reason mutation observer fires 4 times when click. Issue #1083
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…. Still need to run lint and test to ensure that everything working. Issue #1083
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…eight(). experiencing issue where cannot move graph all the way to top nor all the way to left so nodes hidden at top and to the left, which must be something to do with xTranslation + width * graphScale and yTranslation + height * graphScale since that is what controls the top and left borders. Issue #1083
ceciliazaragoza added a commit that referenced this issue Feb 9, 2024
…h when restrict to viewport. Changed graphZoom so that assigned in setGraphZoom. In the equation: Math.floor((Math.abs(xTranslation + width * graphZoom) / .width()) * 1000) !== 0, had to multiply by 1000 instead of 10 which I did before because the ratio of the scale is not inBounds only when that ratio has a significant digit in the thousandths place or less. all lint and test passing. Issue #1083
ceciliazaragoza added a commit that referenced this issue Feb 13, 2024
dondi added a commit that referenced this issue Feb 13, 2024
Issue #1083 D-pad, zoom, and cursor drag now working when graph view restricted to viewport
@dondi
Copy link
Owner

dondi commented Feb 13, 2024

PR has been merged and deployed to beta and feedback has been provided on restrict-graph-to-viewport behavior; the notion is that the graph is the object itself and the d-pad + zoom control that object within the visible rectangle of the browser. @dondi and @ceciliazaragoza will workshop through the full suite of behaviors sometime this week

ceciliazaragoza added a commit that referenced this issue Feb 20, 2024
…e rect changes size if zoom in or out. need to see how can include node edges in box as well
ceciliazaragoza added a commit that referenced this issue Feb 20, 2024
…sure to account for height of node when calculating max x position of node since seems like x position calculated at top left of node
ceciliazaragoza added a commit that referenced this issue Feb 20, 2024
ceciliazaragoza added a commit that referenced this issue Feb 20, 2024
…t) comes first to ensure that boundingBoxContainer border rectangle created before flexibleContainerRect. now can see both rectangles
ceciliazaragoza added a commit that referenced this issue Apr 7, 2024
…specially for the right bound. Also need to alter bounds so that clamp graph correctly when zoomed out too
@ceciliazaragoza
Copy link
Collaborator

ceciliazaragoza commented Apr 9, 2024

I met with @dondi last week to ensure that size of flexBox is being calculated correctly. When move graph to the left, the right bounding border is too wide and left border is too narrow, so need to recalculate right border and take into consideration when left border is in view. When move graph to the right and the right border is in view, sometimes the nodes move past the left border when moving on their own with force graph. Will need to use the xTranslation and graphZoom to see when the left or right borders are in view. Also need to alter bounds so that clamp graph correctly when zoomed out less than 100%.

@dondi
Copy link
Owner

dondi commented Apr 9, 2024

@ceciliazaragoza is homing in on the final values of the thresholds; somewhere along the way, we’ll plan a live demo with @kdahlquist to confirm/calibrate the expected behaviors of the restrict-to-viewport feature

@ceciliazaragoza
Copy link
Collaborator

ceciliazaragoza commented Apr 16, 2024

I committed this morning but did not mention the Issue number. I can now manually change the size of the boundingRectangle, but I cannot get it to match the borders of the viewport consistently. I may consider restructuring graph so that the graph is bounded by the viewport svg itself.

@kdahlquist
Copy link
Collaborator Author

Just make sure that when the restrict graph to viewport is unchecked that the graph would then not be bounded by the viewport.

ceciliazaragoza added a commit that referenced this issue Apr 16, 2024
…m in and out, but when zoom out to the max and zoom in to the max the boundingBoxRect, so need to make sure that the calculation is consistent. Will also work towards making the borders be the boundingBoxRect values as well
ceciliazaragoza added a commit that referenced this issue Apr 16, 2024
…evels. Need to work on right and bottom borders since they are too large
ceciliazaragoza added a commit that referenced this issue Apr 16, 2024
…a negative height or width. probably has to do with the negative x and y values the graph can have now
ceciliazaragoza added a commit that referenced this issue Apr 17, 2024
… top, and bottom bounds restrict the graph to viewport, and when the graph is moved, all of the borders clamp the graph as well
ceciliazaragoza added a commit that referenced this issue Apr 17, 2024
…tomBoundary to take the textWidth and nodeHeight into account as well as the selfReferringEdgeWidth and selfReferringEdgeHeight
ceciliazaragoza added a commit that referenced this issue Apr 17, 2024
…iBox to ensure that the flexbox does not exceed the bounds. need to take the edges into account for flexiBox
ceciliazaragoza added a commit that referenced this issue Apr 17, 2024
…ns for left and top boundaries a function. for some reason now graph exceeds left side of viewport so need to see why that is happening
ceciliazaragoza added a commit that referenced this issue Apr 17, 2024
… I was using BOUNDARY_MARGIN and not BOUNDARY_MARGIN_X_L in tick
ceciliazaragoza added a commit that referenced this issue Apr 18, 2024
…ct. Also removing comments and extra spaces. Also moved viewportBoundsMoveDrag so next to the other flexibleContainer functions
ceciliazaragoza added a commit that referenced this issue Apr 18, 2024
…ct. Also removing comments and extra spaces. Also moved viewportBoundsMoveDrag so next to the other flexibleContainer functions
ceciliazaragoza added a commit that referenced this issue Apr 18, 2024
…g, then the nodes would be able to exceed the viewport bounds when dragged. I changed the viewportBoundsMoveDrag to check that the viewport is in bounds when dragged and now the nodes are always in bounds
ceciliazaragoza added a commit that referenced this issue Apr 18, 2024
…RY_MARGIN_X_L to getLeftXBoundaryMargin and same for get_BOUNDARY_MARGIN_Y_T. Made comments more descriptive and adjust dimensions of flexiBox in calcFlexiBox
@ceciliazaragoza
Copy link
Collaborator

After meeting with @dondi and @kdahlquist today, I fixed styling and solved one edge case. Edge case was when the graph was not moving, it could exceed bounds when dragged, so adjusted viewportBoundsMoveDrag to check when reach bounds when dragged. When restrict graph to viewport all nodes, not edges, are restricted to viewport. Graph can be dragged, zoomed, and moved with Dpad as well.

ceciliazaragoza added a commit that referenced this issue Apr 18, 2024
…tween graphs when restrictToViewport was checked. To fix had to move flexibleContainer variable earlier in code
ceciliazaragoza added a commit that referenced this issue Apr 22, 2024
…deleting unnecessary comment with StackOverflow link since that link ended up not being used
dondi added a commit that referenced this issue Apr 22, 2024
Issue #1083 When check restrict graph to viewport the D-pad and zoom slider do not disappear and graph cannot exceed viewport
ceciliazaragoza added a commit that referenced this issue Apr 23, 2024
…raph was not restricting viewport with Dpad. Fixed by calling viewportBoundsMoveDrag(graphZoom, moveWidth, moveHeight) instead of just (graphZoom, moveWidth, moveHeight) that did not have function name
@kdahlquist
Copy link
Collaborator Author

Confirmed fixed in beta 7.0.15!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants