-
Notifications
You must be signed in to change notification settings - Fork 115
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
[KED-2448] Improve node selected view panning behaviour #374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice one 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
||
// In this example offset is fractional | ||
expect(transform.x).toBeCloseTo(-43.333); | ||
expect(transform.y).toBeCloseTo(6.666); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is quite interesting to see the y values to change by almost 10 times from the original value - is this directly related to the change in the minScaleX
value?
As with my comment in earlier PRs, it might be more clear if the values are represented and compared against a calculated value rather than a specified number that is subject to change whenever we alter any pre-set value, as well as allowing us to truly test the transform rather than going down the easy path of re-plugging numbers whenever a test fails :) WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah these tests are due to be improved, at the moment they are just sanity checks (the fixed constants just allow us to notice when there has been a change of some kind) because the logic is complex - this will be split up later to allow cleaner testing, rather than on the combined result.
# Release 3.9.0 ## Major features and improvements - Add code panel (#346) - Improve view panning behaviour when a node is selected (#356, #363, #370, #373, #374) - Improve layout performance for large graphs (#343) - Save tag state to localStorage (#353) ## Bug fixes and other changes - Improve graph layout code quality, performance and docs (#347) - Update docs to remind on compatibility of Kedro-Viz 3.8.0 with Kedro 0.17 (#367) - Update dependencies (#360, #364, #369) - Fix failing CircleCI build on Windows (#365, #366) - Refactor node-list-row CSS, fixing hover and focus states (#355, #358, #362) - Update iconography (#357, #359) - Fix missing indicator Chrome zoom bug (#349) - Fix sidebar border/box-shadow CSS rules (#351) - Fix server.py to work with versions >0.17 and update contributing docs (#348) - Fix errors when scrolling with empty pipeline (#342) - Ignore coverage on some branches and fix e2e tests (#345) - Fix icon-button tooltips on mobile (#344) - Update SVG-Crowbar to fix errors (#339) - Update contributing docs for new dev server (#341)
Description
Improve selected node view behaviour so panning only occurs when graph is cropped.
Development notes
QA notes
Test by selecting a node at various browser sizes, with various graph sizes, with various open panels.
Check that panning to selected node only occurs when the graph would be cropped by sidebars.
A slight zoom in may occur still when the scale does not meet a minimum.
Checklist
RELEASE.md
fileLegal notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.