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

sankey.states.hover.color is never used #8532

Closed
xuv opened this Issue Jun 26, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@xuv

xuv commented Jun 26, 2018

Expected behaviour

Setting a hover color should display elements in that color when hovered.

Actual behaviour

The hover color is never taken into account

Live demo with steps to reproduce

http://jsfiddle.net/3xkpd5z2/

Product version

6.1.0

Affected browser(s)

Tested in Firefox and Chrome

@sebastianbochan

This comment has been minimized.

Show comment
Hide comment
@sebastianbochan

sebastianbochan Jun 27, 2018

Contributor

Hi @xuv,
Thank you for the reporting. The color is ignored in the pointAttibs function, but I prepared you a wrap which fixes the issue.

  H.wrap(H.seriesTypes.sankey.prototype, 'pointAttribs', function(proceed, point, state) {

    var opacity = this.options.linkOpacity,
    		color = point.color;

    if (state) {
      opacity = this.options.states[state].linkOpacity || opacity;
      color = this.options.states[state].color || point.color;
    }
    
    return {
      fill: point.isNode ?
        color : H.color(color).setOpacity(opacity).get()
    };
  });

Internal note
@TorsteinHonsi does it make sense? If yes I will create a PR.

Contributor

sebastianbochan commented Jun 27, 2018

Hi @xuv,
Thank you for the reporting. The color is ignored in the pointAttibs function, but I prepared you a wrap which fixes the issue.

  H.wrap(H.seriesTypes.sankey.prototype, 'pointAttribs', function(proceed, point, state) {

    var opacity = this.options.linkOpacity,
    		color = point.color;

    if (state) {
      opacity = this.options.states[state].linkOpacity || opacity;
      color = this.options.states[state].color || point.color;
    }
    
    return {
      fill: point.isNode ?
        color : H.color(color).setOpacity(opacity).get()
    };
  });

Internal note
@TorsteinHonsi does it make sense? If yes I will create a PR.

@xuv

This comment has been minimized.

Show comment
Hide comment
@xuv

xuv Jun 27, 2018

@sebastianbochan Thx for the quick response and for the fix.

xuv commented Jun 27, 2018

@sebastianbochan Thx for the quick response and for the fix.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jun 28, 2018

Collaborator

@sebastianbochan Yes, makes sense! Thanks!

Collaborator

TorsteinHonsi commented Jun 28, 2018

@sebastianbochan Yes, makes sense! Thanks!

@mjkstudios

This comment has been minimized.

Show comment
Hide comment
@mjkstudios

mjkstudios Sep 13, 2018

I had been using the select state to change the opacity on selection. That's great that the color is now taken into consideration, but for anyone wanting the previous behavior, color must be explicitly set to a falsy value:

http://jsfiddle.net/j2Lyzt1k/

    states: {
      select: {
      	color: null, // defaults to #cccccc
        linkOpacity: 1
      }
    }

mjkstudios commented Sep 13, 2018

I had been using the select state to change the opacity on selection. That's great that the color is now taken into consideration, but for anyone wanting the previous behavior, color must be explicitly set to a falsy value:

http://jsfiddle.net/j2Lyzt1k/

    states: {
      select: {
      	color: null, // defaults to #cccccc
        linkOpacity: 1
      }
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment