plotly / plotly/plotly.js

plotly_click handler return value is ignored, no way to cancel event processing from custom handler

Open
#5,477 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug P3
Dominant language
JavaScript
Stars
18.3k
Forks
2k
Avg merge
2d 12h
Merged PRs (30d)
28

Description

I'm currently using a plotly_click custom handler (via react-plotly onClick prop) on a treemap, where I would like to cancel the default zoom-in / zoom-out behaviour if the user is pressing control.

For plotly_legendclick for example, the convention is to return false from the custom handler, which will stop default event processing. See https://community.plotly.com/t/disable-legend-click-functionality-hiding-traces/1345/4 for @alexcjohnson 's reply in this regard.

However, for the plotly_click event I've traced the code into this little section in plotlyjs attachFxHandlers.onClick():

        var clickVal = Events.triggerHandler(gd, 'plotly_' + trace.type + 'click', typeClickEvtData);

        if(clickVal !== false && fullLayoutNow.hovermode) {
            gd._hoverdata = [makeEventData(pt, traceNow, opts.eventDataKeys)];
            Fx.click(gd, d3.event);
        }

// 2 lines ommitted

        // if custom handler returns false, we're done!
        if(clickVal === false) return;

So the event for plotly_treemapclick in this case will honour the convention as can be seen from the clickVal = ..., but the standard plotly_click handler is invoked by the Fx.click() invocation, which ignores the handler return values.

Unfortunately, the call chain from Fx.click() is quite deep: onClick() -> click() -> emitClick() -> plotObj.emit() -> emit() -> emitOne() -> customHandler. To bring back the return value is going to be quite the adventure. Also, at some spots along the chain, return values are used for a different purpose.

Alternatively, the code in the uppermost onClick() could for example check event.defaultPrevented -- this is not quite the Plotly convention, but then again that convention is not very well documented at all, and this would be far more straight-forward to implement rather than bringing back the custom handler return value. Furthermore, many javascript programmers know to look for preventDefault.

In addition, this fix would immediately also work from React.

What do people think about this?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at attachFxHandlers.onClick(), then trace the documented Fx.click() chain through click(), emitClick(), plotObj.emit(), emit(), and emitOne(). Compare the plotly_treemapclick return handling with the standard plotly_click path and determine how cancellation should propagate. Done means a false-returning or otherwise supported custom handler can prevent default click processing, including through React.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react
Domain
data-visualization, frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.