feat(Popup): add 'offset' prop

[kuzhelov]

This PR introduces offset prop to the Popup API that would allow client to fine-tune position of the rendered popup. Effect of this prop's value is intuitive to the client, RTL mode is also respected.

TODO

• update changelog

Values supported

• px or unit-less, interpreted as pixels
• %, percentage relative to the length of the trigger element
• %p, percentage relative to the length of the popup element
• vw, CSS viewport width unit
• vh, CSS viewport height unit

Example

<Popup
   trigger={...}
   position='above'
   align='start'
   offset='100%'
/>

No Offset (align: start)

With Offset (align: start, offset: 100%)

---

Fixes #313

[mnajdova]

Too bad applying modifiers is the only way we can adjust the offset... There is no way we can use this prop inside the styles in order to achieve the position, right? It really feels awkward to have to tackle rtl in the component's logic..

[kuzhelov]

yes, there is no any - so, there are several reasons for that.

First is simple - there are no additional props of Popper that would allow client to customize positioning (actually, good thing). And here we are getting to the next alternative option that might come to mind (the one you've mentioned) - use styles for influencing these offsets.

I've tried this one - and have done that only to clearly see that even in LTR mode taken in isolation we have lots of issues. As an example: if it is about 'start' value for align property, which corresponds to popup aligned with the left side of trigger,

we might use left to specify an offset (as absolute positioning strategy is used by Popper). However, when we will achieve necessary result and switch to the ride sides alignment, we will see that Popper is using transform property to ensure alignment for the right sides, and here we cannot use offsets of left-right CSS properties anymore.

For the sake of not preventing havoc to wreak our code, decided to just reuse solution Popper has already introduced for solving exactly this problem (probably, the most reasonable thing) - and this, in fact, what we've done with the modifiers. Note, we are not abusing it - we are using the tool from Popper that is, actually, dedicated to solve this exact problem :)

[mnajdova]

👍 for the functionality... Let's try to move somehow the offset in the styles if it is possible before merging (I don't think it is, but I had to mention it).

[kuzhelov]

moving offset to the styles is possible, but it will lead to the following reasonable question:

why we have offset in the styles, and align and position as Popup API? Both of them are responsible for defining position of the popup, so it should be either both are defined in styles, or both are defined in the API.

To me it seems reasonable to have this being part of the component's API (the same reasoning for having color prop in Divider's API applies here) - and this is what we have now. More than that, just to prove our way of thinking about it, we might take a look on other libs, especially on SUIR - here the same approach (use API for positioning) is taken.

So, while understand your concerns, feel that have both as part of component's API is preferred.

[miroslavstastny]

@kuzhelov - sorry that I hadn't managed to comment on this before merging.
These would be my comments:

1. Add example which sets both horizontal and vertical offset at the same time.
2. Computed offset breaks in RTL.

Code:

{ position: 'above', align: 'start', offset: '-100%p + 50%, 10px', rotateArrowUp: '-45deg' },

LTR:

RTL:

3. It's not possible to set offset bigger than trigger size

Code (200px offset vs 80px trigger):

{ position: 'above', align: 'start', offset: '200px, 10px', rotateArrowUp: '-45deg' },