graphviz-react icon indicating copy to clipboard operation
graphviz-react copied to clipboard

can't disable zoom after it has been enabled

Open lnavarette opened this issue 5 years ago • 6 comments

I am trying to create a button to enable/disable zoom because if you're trying to scroll down the page, the zoom will sometimes get triggered instead depending on where your mouse is.

Here's a component which reproduces this.

function GraphWithLockButton() {
    const [locked, setLocked] = useState(true);
    return (
        <div id={'graph-container'} style={{position: 'relative'}}>
            <button onClick={e => setLocked(!locked)}
                    style={{position: 'absolute', left: 5, top: 5}}>
                {locked ? 'locked' : 'unlocked'}
            </button>
            <Graphviz options={{zoom: !locked, width: 300, height: 300}} dot={`digraph {a -> b}`}/>
        </div>
    )
}

If the initial state of locked is true,

  • Zoom starts off disabled (as expected).
  • When you click the button, zoom becomes enabled (as expected)
  • When you click the button a second time, state flips to locked, but zoom is still enabled

If the initial state of locked is false,

  • Zoom starts off enabled (as expected).
  • When you click the button, state flips to locked, but zoom is still enabled

Tested on both 1.0.6 and 1.1.1

lnavarette avatar Dec 20 '20 20:12 lnavarette

Hi @lnavarette, I've noticed this behaviour as well. My gut feeling is that this is potentially a bug from the d3-graphviz package (I say potentially because it might be the intended behaviour per the documentation that zoom can't be disabled after being enabled, I'm not sure).

Given though that d3-graphviz has moved up a major version and this project is still tracking the older version (because of breaking changes) any bug fix in d3-graphviz may not find its way into this project immediately (depends if bugs get back ported to older versions or not).

I'll have a look and see if it can be patched directly in this project.

DomParfitt avatar Dec 22 '20 11:12 DomParfitt

thanks @DomParfitt - I forked your component hoping i could just explicitly enable/disable using d3-graphviz directly but that didn't work either, so I filed an issue there as well.

https://github.com/magjac/d3-graphviz/issues/180

Seems like it might be possible using d3-zoom directly but I figured I'd see if anyone who knows more about d3 could point me in a better direction first before venturing down that path.

lnavarette avatar Dec 22 '20 23:12 lnavarette

@lnavarette I've left a comment on the issue on d3-graphviz with what I think the cause is. If that is the issue then I'm not sure whether any fix for it will be back ported to v2 or will just be included in v3. As this project is currently using v2 explicitly then there may still be a need for a workaround until we can move to v3.

In terms of a workaround I was thinking that the current renderGraph method could add a check for a change in the zoom setting and recreate the whole graphviz instance if necessary (instead of just passing the updated options). I haven't tried it yet to see if it is feasible or not though.

DomParfitt avatar Dec 23 '20 09:12 DomParfitt

Trying that didn't seem to work for me, but your comment on the d3-graphviz project did help me find a different workaround. It's admittedly quite hacky.

        const gv = graphviz(`#${id}`);

        gv.options({
            fit: true,
            width: graphWidth,
            height: 300,
            zoom: true // always enable zoom on draw to init the zoom behavior
        }).renderDot(props.digraph);

        // then enable/disable zoom based on options
        const svg = select(`#${id} svg`);
        if (locked) {
            svg.on('.zoom', null);
        } else {
            svg.call(gv.zoomBehavior() as any);
        }

lnavarette avatar Dec 26 '20 21:12 lnavarette

I've supplied a PR that fixes this upstream which uses a similar approach as @lnavarette's above. You might be able to backport it it to v2 or make a similar solution in graphviz-react using graphviz.zoomSelection() instead of relying on the svg id.

magjac avatar Jan 09 '21 12:01 magjac

I have the same problem

TakhirMirza avatar Feb 12 '22 03:02 TakhirMirza