React-popper: Dynamic positioning arrow incorrect after upgrade to 0.7.1

[rvk86]

After upgrading to 0.7.1 the dynamic positioning of the arrow became incorrect. Depending on the chosen placement, it adds a top or left attribute to the arrow so it's contained inside the popper element. It even happens with the most basic readme example together with the css stylesheet provided by popper itself (https://github.com/FezVrasta/popper.js/blob/master/docs/css/popper.css):

<Manager>
    <Target style={{ width: 120, height: 120, background: '#b4da55' }}>
      Target Box
    </Target>
    <Popper placement="left" className="popper">
      Left Content
      <Arrow className="popper__arrow"/>
    </Popper>
    <Popper placement="right" className="popper">
      Right Content
      <Arrow className="popper__arrow"/>
    </Popper>
  </Manager>

[souporserious]

@FezVrasta and @giladgray any idea on this? I noted it in the CHANGELOG, I thought that is how it needed to be with the recent changes to how top/left is applied.

[FezVrasta]

I don't think I get where the problem is. Is it possible to have a working demo?

[souporserious]

I think the problem is that top and left are always applied now, whereas before it checked if it was available or not. So this means that it can be top: 0 or left: 0 now, which either requires an !important rule to override the style, or using margin instead of top/left to position the arrow. Does that make sense?

[FezVrasta]

Umh yeah it does. It looks like a bug of Popper.js, it should set the value to false to actually unset it.

[souporserious]

Ah interesting! I always thought it was weird that it applied it, that's why I had that conditional in there. I'll leave this open until it is fixed in Popper.js. If I get a chance, I'll try and make a PR with a fix if I can find the source of the problem.

[FezVrasta]

Oops sorry, on Popper.js the other side is properly unset... Can it be something about the way how React handles '' as value? I use an empty string to unset the values.

[souporserious]

Ah I see. From what I remember, data comes back with a 0 value for the opposite side, so that's why it gets set. I'll look into it to confirm that's what's happening.

[FezVrasta]

In Popper.js it gets unset:
https://github.com/FezVrasta/popper.js/blob/master/packages/popper/src/modifiers/arrow.js#L76

[souporserious]

Ah 💩 I found it. It's the use of the shorthand to convert anything to a number +top and +left, which is turning the empty string to a 0.

[FezVrasta]

I fucking love JavaScript

[souporserious]

Hahaha right?! I think I've _finally_ learned not to be clever even when a language allows it. Fixed in the latest release https://github.com/souporserious/react-popper/commit/ef520ee1213f30d3af3f10c6f5b4a04a888deb73 thanks peeps! 🎉

[rvk86]

Thanks for jumping into this guys, I can confirm it works now :)