gilbarbara / react-inlinesvg

An SVG loader component for ReactJS
https://codesandbox.io/s/github/gilbarbara/react-inlinesvg/tree/main/demo
MIT License
1.27k stars 101 forks source link

Assigning svg a className or attribute #4

Closed ChrisSki closed 10 years ago

ChrisSki commented 10 years ago

@matthewwithanm is there a way to give the actual svg a className or attribute? Reason being when I use something like GSAP for tweening, it's necessary for me to reference the actual svg for animation.

matthewwithanm commented 10 years ago

Hm…we could add that feature, but I'd kind of like to keep mucking around with the SVG contents to a minimum. Is it not possible to just give the class to the isvg wrapper and target it by using .myclass > svg or similar?

ChrisSki commented 10 years ago

I've tried to do that, but since we're keeping jQuery to a minimum I'm finding it troublesome to select the actual svg with vanilla javascript. I've tried selecting the parent class with getElementsByClassName() and then getElementsByTagName(), as well as querySelectorAll(), with no luck. Any more code than that I feel like a simpler solution should be available. Also, I think the ability to add an ID may be necessary to some as well. Just my .02 cents here :)

ChrisSki commented 10 years ago

As a solution, I used the onLoad callback function to call my tweening functions instead of componentDidMount(), because the childNodes had not been rendered at the time of mounting. This works perfectly!

ChrisSki commented 10 years ago

@matthewwithanm first, sorry for the repetitive posting, but I thought you might like to know about this scenario. Whether it be an edge case or not, it might be something to look into since animating things with React is not the easiest thing at the moment, and good solutions are helpful. Building a great solution for svgs and animation could be a huge help to many people. I think this plugin is super close.

Ok. So here's an issue with my previous solution. When using something like GSAP TimelineLite to tween multiple elements in succession, the engine is looking for nodes to manipulate. In this case, the nodes are not there when componentDidMount() fires. So using the onLoad callback you provide works well, but only when you have one svg to manipulate, like the following example:

loadSvg: function () {

    var logoDiv = document.getElementsByClassName("onboarding-logo-container");
    var logo = logoDiv[0].getElementsByTagName("svg");

    var tl = new TimelineLite();
    tl.to(text, .25, {autoAlpha: 1})
      .to(text1, .25, {autoAlpha: 1}, '-=0.25')
      .to(logo, .50, {autoAlpha: 1, y: 22})
      .to(button, .5, {autoAlpha: 1, delay: 1.25});
  },

  render: function() {

    return (
      <div className="welcome-container">
        <div className="welcome-text-container">
          <span className="welcome-text" ref="welcome">Thanks for installing</span>
          <isvg className="onboarding-logo-container" onLoad={this.loadSvg} src={chrome.runtime.getURL("common/images/icons/logo.svg")} />
          <span className="welcome-text" ref="welcome1">Your new source for trusted information.</span>
        </div>
        <div className="get-started" ref="started" onClick={this.start}>
          <div className="get-started-btn">Let's Get Started</div>
        </div>
      </div>
    );
  }

In the following example, I have two svgs that I need to animate within a single TimelineLite tween. I have the following code that works just fine, but it doesn't seem very intuitive. Take notice that what I am doing is actually giving each isvg a counter when each fires the onLoad callback. If it's greater than or equal to 2, fire everything off.

loaded: 0,

loadSvg: function () {
    this.loaded = this.loaded + 1;
    if (this.loaded >= 2) {
      var arrow = this.refs.arrow.getDOMNode();
      var text = this.refs.text.getDOMNode();
      var text1 = this.refs.text1.getDOMNode();
      var logoDiv = document.getElementsByClassName("get-access-logo-container");
      var logo = logoDiv[0].getElementsByTagName("svg");

      var tl = new TimelineLite();
      tl.to(text,.25, {autoAlpha: 1})
        .to(text1,.25, {autoAlpha: 1}, '-=0.25')
        .to(logo,.50, {autoAlpha: 1, rotationX: 0})
        .to(arrow,.5, {autoAlpha: 1, rotation: -10, x:0, y: -80, delay: .75});
    }
  },

  render: function() {
    return (
          <div className="get-access-container">
            <isvg className="get-access-arrow" onLoad={this.loadSvg} ref="arrow" src={chrome.runtime.getURL("common/images/onboarding/arrow.svg")} />
            <div className="get-access">
              <span className="get-access-text" ref="text">click on the</span>
              <isvg className="get-access-logo-container" onLoad={this.loadSvg} src={chrome.runtime.getURL("common/images/icons/logo/icon.svg")} />
              <span className="get-access-text" ref="text1">icon in the toolbar</span>
            </div>
          </div>
          );
}

Note: Removing the counter does seem to work, but it doesn't feel right firing the same function twice and expecting everything to work perfectly. Maybe I'm just wrong or over thinking this?

matthewwithanm commented 10 years ago

I see. There doesn't seem to be anything wrong with this approach, though you could also do something more robust. There are a bunch of node libraries for doing stuff like this or you could resolving promises in the onLoad callbacks and use Promise.all. Either way, I think this is an async flow control issue and not something that can be solved with inlinesvg, right? You'd have the same issue if you wanted to, say, wait for two images to load.

ChrisSki commented 10 years ago

After thinking more about it, you're correct. I think dealing with multiple tweens, or any other object, will be handling them with something like Promises.

matthewwithanm commented 10 years ago

Cool! Should we close?

ChrisSki commented 10 years ago

Yeah, I think so.