Fire perspective-config-update event on D3FC legend and axis events
#1748
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jkusa
reviewed
Mar 9, 2022
|
|
||
| pinned = unpinNodeFromTopRight(node, pinned); | ||
| drag.on("end", function (event) { |
There was a problem hiding this comment.
👍 Thanks for making this update. It did cross my mind that we probably want to fire the event after the user stops updating the position, but wasn't 100% sure how to do that here.
| // Await the table load | ||
| await viewer.getTable(); | ||
|
|
||
| viewer.restore({ |
There was a problem hiding this comment.
Suggested change
| viewer.restore({ | |
| await viewer.restore({ |
Good catch on the test above. Probably want to do the same here.
There was a problem hiding this comment.
Actually this was more nefarious than it appears - turns out that this test had a similar issue, mouse.down() was called to initiate the drag in the test, but not mouse.up() which fires the drag end event. The test "worked" by asserting the perspective-config-update which was fired by restore().
texodus
commented
Mar 9, 2022
| @@ -125,6 +125,7 @@ utils.with_server({}, () => { | |||
| await page.mouse.move(start.x, start.y); | |||
| await page.mouse.down(); | |||
| await page.mouse.move(target.x, target.y); | |||
| await page.mouse.up(); | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebased and fixed-up version of #1732. Fixes #1729. Changes from #1732
dragtoend. For many apps which use<perspective-viewer>, the operations which listen for theperspective-config-updateevent may be relatively expensive, e.g. sending an event to a server, so we want to generally conserve these to applicable changes. With this change, only 1 event is triggered when the drag operation completes, as opposed to 100s while the legend is moved.scatter.htmlandtreemap.htmlI think was introduced in Virtual column selector #1722 - this PR does not modify these results (as validated in the reference HTML and PNG output I have from previous runs), so theseresults.jsonchanges are reverted..restore(), then clicks on the parent element of the axis label which has the click event listener. The result is, the event for the axis switch is not fired, but the.restore()methods which is not awaited sometimes does, causing this test to flake. Fixed byawait-ingrestore()and changing the click selector to pick the label itself.● events.html › perspective-config-update event is fired when series axis is changed Node is either not clickable or not an HTMLElement 59 | ) 60 | ).asElement(); > 61 | await axisLabel.click(axisLabel); | ^ 62 | 63 | const count = await page.evaluate(async (viewer) => { 64 | // Await the plugin rendering