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

First attempt at a click anywhere #5416

Closed
wants to merge 0 commits into from
Closed

Conversation

sleighsoft
Copy link
Contributor

I unfortunately messed up the repository and had to recreate my PR.

This is a reference to #5138

@sleighsoft
Copy link
Contributor Author

sleighsoft commented Jan 16, 2021

@sleighsoft
Copy link
Contributor Author

sleighsoft commented Jan 16, 2021

And when running npm run test-jasmine -- test\jasmine\tests\click_test.js --FF --nowatch in the plotly.js repo 18 tests fail if I select the latest commit (without my changes).

@sleighsoft
Copy link
Contributor Author

Also, is there a good place to add a demo?

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <script src="../plotly.js/dist/plotly.js"></script>
</head>

<body>
    <div id="tester" style="width:600px;height:250px;"></div>
    <script>
        var TESTER = document.getElementById('tester');
        Plotly.newPlot(TESTER, [{
            x: [1, 2, 3, 4],
            y: [4, 1, 3, 5],
            mode: "lines+markers",
            marker: {size: 12},
        }], {
            margin: { t: 0 },
            clickmode: "event+anywhere"
        });
        TESTER.on('plotly_click', function(clickData) {
            var x = clickData["points"][0]["x"];
            Plotly.relayout(TESTER, {
                shapes: [
                {
                    type: "line",
                    x0: x,
                    y0: 0,
                    x1: x,
                    y1: 1,
                    yref: "paper",
                    line: {color: "RoyalBlue", width:3},
                }
                ]
            });
        });
    </script>
</body>
</html>

@archmoj
Copy link
Contributor

archmoj commented Jan 16, 2021

@nicolaskruchten Any idea why the following happens when running python setup.py updateplotlyjsdev --devbranch pull/5416

    ERROR in ./node_modules/plotly.js/dist/plotly.js
    Module build failed: SyntaxError: Unexpected character '�' (5879:32)

It looks like the build artifact has a weird encoding? (I am on Windows 10)

Similar to #5382
Possibly would be fixed by d3-geo cc: #5112

@archmoj
Copy link
Contributor

archmoj commented Jan 16, 2021

I unfortunately messed up the repository and had to recreate my PR.

Thanks for the new PR.
In order to be able to sync the master branch of your fork with upstream master, next time I suggest you open PRs from another branch on your fork (not the master).


// why do we get a double event without this???
if(evt.stopImmediatePropagation) evt.stopImmediatePropagation();
}
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a new line to the end of this file.
To catch syntax errors you could run
npm run test-syntax && npm run lint

@archmoj
Copy link
Contributor

archmoj commented Jan 16, 2021

And when running npm run test-jasmine -- test\jasmine\tests\click_test.js --FF --nowatch in the plotly.js repo 18 tests fail if I select the latest commit (without my changes).

Thanks for running them locally.
Some tests are not stable and fail locally; but they all passed on the CI.

@sleighsoft
Copy link
Contributor Author

sleighsoft commented Jan 21, 2021

I unfortunately messed up the repository and had to recreate my PR.

Thanks for the new PR.
In order to be able to sync the master branch of your fork with upstream master, next time I suggest you open PRs from another branch on your fork (not the master).

Thanks for pointing that out. I have been in a hurry and must have missed that... again. Should I open another new PR to be able to stay in sync with the plotly:master? I guess I've to wait for your geo fix to pass to be able to run python setup.py updateplotlyjsdev

@archmoj
Copy link
Contributor

archmoj commented Jan 21, 2021

The bundles are fixed on master after #5426.
So you don't have to wait for the geo PR.

@sleighsoft
Copy link
Contributor Author

sleighsoft commented Jan 24, 2021

Moved code to a new branch on my end to keep up with changes in master: PR is now at #5443

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

Successfully merging this pull request may close these issues.

2 participants