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

Clicking on a link only works directly on the node text #891

Closed
GDFaber opened this issue Jul 23, 2019 · 9 comments · Fixed by #1163
Closed

Clicking on a link only works directly on the node text #891

GDFaber opened this issue Jul 23, 2019 · 9 comments · Fixed by #1163
Assignees
Labels
Area: Development Graph: Flow Retained Nonperishable Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect

Comments

@GDFaber
Copy link
Member

GDFaber commented Jul 23, 2019

Describe current behaviour
When an URL click event is defined for a flowchart node, it is only called when the node text is clicked. Clicking anywhere else in the node does not trigger the click event. The mouse pointer suggests that the node could be clicked at any position though.

To Reproduce
This can be reproduced using the interaction example at http://knsv.github.io/mermaid/#/flowchart?id=interaction

Expected behavior
Link/function should be called no matter what position a user has clicked on the node.

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version: 75
@GDFaber GDFaber changed the title Click event only works by clicking on node text Clicking on a link only works directly on the node text Aug 3, 2019
@IOrlandoni IOrlandoni added Area: Development Graph: Flow Contributor needed Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect Retained Nonperishable labels Sep 26, 2019
@jgreywolf
Copy link
Contributor

@GDFaber

Could you please reproduce the issue in https://mermaidjs.github.io/mermaid-live-editor/ and post a link back here? In my testing I am not seeing a big distinction between when the hover happens for the link/action

@GDFaber
Copy link
Member Author

GDFaber commented Nov 20, 2019

Hi @jgreywolf, I have provided a minimal example here:

https://mermaidjs.github.io/mermaid-live-editor/#/edit/eyJjb2RlIjoiZ3JhcGggVERcbkN7TGV0IG1lIHRoaW5rfVxuY2xpY2sgQyBcImh0dHA6Ly93d3cuZ29vZ2xlLmNvbVwiXG4iLCJtZXJtYWlkIjp7InNlY3VyaXR5TGV2ZWwiOiJsb29zZSIsInRoZW1lIjoiZGVmYXVsdCJ9fQ

My point is that you should be able to click on the node on any position to open the URL, but that does only happen when you click on the link directly. But I don't mind if this is considered as the expected behaviour. If so, we might as well close the issue.

@IOrlandoni
Copy link
Member

My point is that you should be able to click on the node on any position to open the URL, but that does only happen when you click on the link directly. But I don't mind if this is considered as the expected behaviour. If so, we might as well close the issue.

I agreed with that, so I left it as a bug. If not, we'll move it over to "enhancement".

@jgreywolf
Copy link
Contributor

Thanks @GDFaber When I tried to reproduce with a rectangle there was hardly any space left over to see the issue :)

@jgreywolf
Copy link
Contributor

After further investigation I am not 100% sure if this is something that can be addressed within mermaid - or at least not as just a bug?.

If I am following the code correctly, when an href link is found it is added to the object as a property called link. Eventually this is associated with a ForeignObject that represents the vertexNode for the label text. This is then provided as an argument label to the g.setNode method of graphLib. Somewhere the ForeignObject is added as a child of the <a href...> tag.

What I am not finding is a good place to add the action to a parent element of the resulting DOM element

From what I can tell, if the action is a callback it follows a different path, and the eventListener is added to the entire shape so it should operate as expected. Im not sure how to add a script to handle a callback in the live editor though, so not able to test there for sure.

Im not sure how to continue right now.

@IOrlandoni
Copy link
Member

@jgreywolf I'm thinking about this from a different perspective and I might be completely wrong but here it goes:
Wouldnt stretching the <a> so it fills the node solve this issue in a nice way? Instead of also making the node clickable, just filling it with the clickable element.

@jgreywolf
Copy link
Contributor

No, that is what I was thinking as well, I just have not been successful in making that happen. When the click event is just a link the url is added to the object, and at render time a svg link element is generated, the shape text is added as a child of the linkElement, and the linkElement (which has no parentElement yet) - is provided as the label parameter to the g.setNode function call to generate the shape. ...

Basically, from what I can tell, these aren't added into the DOM until after D3 renders the SVG element...

@GDFaber
Copy link
Member Author

GDFaber commented Nov 28, 2019

Just for the record I leave my workaround here, maybe it can be improved or turned into something similar to be worth integrating.

$('#flowchart svg g.clickable').each(function () {
   $(this).click(function (event) {
      var link = $(this).find('a');
      if (event.detail == 1) {
         // open link in new window
         // maybe do other things as well
      }
   });
});

I do this after rendering, "#flowchart" is my rendering container (I'm only using flowcharts at the moment).

@GDFaber
Copy link
Member Author

GDFaber commented Dec 25, 2019

I'm currently working on a PR for this.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: Development Graph: Flow Retained Nonperishable Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants