"OnElementSelected" is not fired on props change

[amirhouieh]

Facing same issue as in old-version. While the wrapper component is updated with new props, but onElementSelected is only called once!
here is my component render method:

  render() {
    const {currentIndex} = this.props;
    const selector = currentIndex === 1 ? "g": "path";
    return (
        <div className={style.iconWrapper}>
          {currentIndex}
          <SvgLoader path={"/persons.svg"}>
            <SvgProxy selector={selector}
                      onElementSelected={(nodes) => {
                        console.log(nodes);
                      }}/>
          </SvgLoader>
        </div>
    )
  }

[amirhouieh]

This seems to be fixable by either forcing update method here
or there is something wrong here with this condition

[hugozap]

Proxies are currently not meant to change target elements by changing selectors. However you don't need to do it. If you need to update another element you can use an array and just add another item to it

<SvgLoader...>
 {elementsToUpdate.map( elem => <SvgProxy key={elem} selector={elem} fill={blue}/> )}
<SvgLoader>

The reason changing selector for existing proxies is not supported is that if you change your selector to other element then things get complicated as someone would expect that the first selected element state would be reseted, so we would need to keep track of the original attribute values.

[amirhouieh]

I see what you mean. But how this approach would work in an occasion where the selector is something like this:

    const selector = "svg>g";

This way you expect to build up queries for each selector kind of manually? like:

    const elementsToUpdate = ["svg>g:nth-child(0)", "svg>g:nth-child(1)", ...]

?

[hugozap]

Right now onElementSelected will be called with the array of elements if there is more than one. And all of them will be updated. Now that I think about it, supporting selector change may not be that hard but first Id like to get some feedback on what should happen to the first selection. Should they be reset or kept their updated values? Any thoughts on this? El lun., 2 de jul. de 2018, 4:02 a. m., amir houieh < notifications@github.com> escribió:

…

I see what you mean. But how this approach would work in an occasion where the selector is something like this: const selector = "svg>g"; This way you expect to build up queries for each selector kind of manually? like: const elementsToUpdate = ["svg>g:nth-child(0)", "svg>g:nth-child(1)", ...] ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAkB1_TvdqbSUeFhReq13ZOqPMpJOIE3ks5uCeGZgaJpZM4U5kAz> .

[amirhouieh]

I am not sure if I fully understood why you would not want to overwrite the state of all selectors even if they are not changed! Actually right now I am forcing the updateSvgElements and it works perfectly.

Anyway I would recommend to trigger the updates for all the selectors, and leave the stage updating for whoever uses the library.

[amirhouieh]

I am leaving my render method here, to just showcase what I am exactly doing and what I am talking about

render(){
 const selector = `svg>g`;

  return(
  <SvgLoader path={path}>
                <SvgProxy selector={selector}
                          onElementSelected={(nodes) => {
                            if(nodes.length){
                              //expect always to first group be the background so we exclude it
                              const background = nodes.shift();
                              this.applyColorToShape(background, backgroundColor);

                              nodes.forEach((node, i) => {
                                if (i === this.props.currentIndex) {
                                  node.classList.add("highlighted");
                                  [].slice
                                      .call(node.querySelectorAll("circle,path,polygon,polyline,line"))
                                      .forEach(shape => {
                                        this.applyColorToShape(shape, this.props.iconColor)
                                      })
                                } else {
                                  node.classList.remove("highlighted");
                                }
                              })
                            }else{
                              console.log("invalid SVG format, expect at-least two groups");
                            }

                          }}/>
              </SvgLoader>
  )

}

[hugozap]

Thanks @amirhouieh I added the ability to change selector to SvgProxy. When the selector changes, the original elements will kept the updated attributes and the new selection will be updated accordingly.

[amirhouieh]

@hugozap Thanks for the fix, but actually that does not help my problem :).

I think there is a misunderstanding. the example I made in the first note was a bad example. It shows a change in selector while this was not my intention and initial issue. The problem I was facing and I still do (in v1.1.6) is simply the title of this issue: "OnElementSelected" is not fired on props change.

The use-case here is that I have a higher order component which controls some states (let say the color) of SVG component. so I expect whenever the state changes, I can receive it as a prop in SVG and update it accordingly.

[AnnickNijm]

I have the same issue.

I use OnElementSelected to dynamically define the content of some inside my svg but it isn't updated when prop is updated.

Code snippet (if there are typo, they're not relevant, it's a condensed version of my actual code which is properly displaying all the info):

<SvgLoader path={bodysvg} style={{width:"100%", height:"auto"}}>
     <SvgProxy selector="#haircolor-input" onElementSelected={() => {
         document.getElementById('haircolor-input').textContent= this.props.character.haircolor}
     }/>
</SvgLoader>

To be honest it's kind of a major issue for me so I hope there's either a workaround or a fix soon I really love the plugin otherwise.

[lynnerang]

Also having this issue right now, wanting to select all of a specific class and add an additional class to only half of them. Any workarounds I can try?

[hugozap]

@lynnerang You can add a Proxy that targets all the relevant elements with the default class value and another Proxy where the selector targets the half you want to add the class to.

[lynnerang]

Sounds awesome, is there anywhere you can point me to on more documentation of how the Proxy works? I'd love to essentially be able to loop through index 5-10 of a total of 10 items selected by a class and add another class to those, but having a really difficult time figuring out all the capabilities of SvgProxy or even example use cases of it. I've been able to get a hold of the 10 nodes of the original class through the onElementSelect method, but it seems like adding classes or data attributed to those doesn't have any effect.

[hugozap]

Sounds awesome, is there anywhere you can point me to on more documentation of how the Proxy works? I'd love to essentially be able to loop through index 5-10 of a total of 10 items selected by a class and add another class to those, but having a really difficult time figuring out all the capabilities of SvgProxy or even example use cases of it. I've been able to get a hold of the 10 nodes of the original class through the onElementSelect method, but it seems like adding classes or data attributed to those doesn't have any effect.

In this example there are 5 rects and the proxy selects the elements after the third one: https://codesandbox.io/s/react-svgmt-nth-child-vyn7d

When a proxy element is rendered, its selector property is read and a query retrieves all the elements that match the selector, then all the proxy attributes are applied to each of the matched elements. (Previous attribute values are not remembered, so if you remove the proxy the changes will still be there).

[hugozap]

FYI I started work on the next version of react-svgmt. Correct behavior for proxies that change selectors will be included, I'll post updates to this thread

[lynnerang]

This is exactly what I needed, it works beautifully. Thank you so much!

[lynnerang]

I'm guessing this is a tall order considering CSS doesn't allow this, but any chance there's a way to so something along the lines of "nth-match" or restrict to certain indexes of what was selected when selecting by class with SvgProxy?

[hugozap]

I'm guessing this is a tall order considering CSS doesn't allow this, but any chance there's a way to so something along the lines of "nth-match" or restrict to certain indexes of what was selected when selecting by class with SvgProxy?

If a combination of nth-child doesn't work (I see that there's more advanced queries supported by combining nth-child http://nthmaster.com/) then you can do it with code by creating a list of the valid selectors and then creating a proxy for each one, check this example: https://codesandbox.io/s/react-svgmt-nth-child-rh48e

[lynnerang]

Thanks so much for all the help, I ended up getting what I needed done. I really appreciate this library, it was the only thing I could find that would let me select child elements of an imported svg in the way I needed to!

[hugozap]

@lynnerang That's awesome! I realized that more tutorials and better docs would be helpful, I plan to improve things in future releases, thanks for the questions/feedback!