thednp / bootstrap.native

Bootstrap components build with Typescript
http://thednp.github.io/bootstrap.native/
MIT License
1.7k stars 177 forks source link

Popover overflows bottom #455

Closed jcorporation closed 1 year ago

jcorporation commented 1 year ago

I open an issue to track it better, I hope that's ok.

Reference:

Latest changes for v5.0.0-alpha2, has no effect in my tests. The popover is still positioned not correctly.

image

thednp commented 1 year ago

I have an idea on how to improve this, will let you know asap, perhaps tomorrow.

thednp commented 1 year ago

@jcorporation I've played with the code a bit, I made it work to take this case into account and well... it's not right, mostly because the new positioning system based on position fixed for the tooltip and a much more simple algorithm makes Popovers always stick to the viewport edges, even if their trigger elements are not in the viewport at all. In short, it solves one niche case and produces an even bigger problem for everything else.

In my mind a solution can be to use a custom CSS class with a top: YOUR_VALUE !important position for your Popover and similar for it's arrow, since this is a very specific use case. It might also help us to implement the inserted.bs.popover event (it shouldn't be hard at all) to update your Popover position however you want when this event triggers. Even better we can also add updated.bs.popover but I think all this should benefit the community at large.

What do you think?

Happy New Year!!

jcorporation commented 1 year ago

This additional events should solve my problem. Could you add this events also to the dropdown component?

As reference my current patch to solve this: https://github.com/jcorporation/bootstrap.native/commit/73aa96402667ceb8e75fcc4166bf763d28d361a9#diff-73e8e7ae57dcf21616bd61f7eac8e8190db171f06e1f908cd7ae724b95b9c379

thednp commented 1 year ago

Let me finish updating the tests and I'll see what I can do. Meanwhile let me know what you think regarding:

What do you think?

jcorporation commented 1 year ago

We can use only an updated event. It should trigger after the position styles are applied and before the dom becomes visible.

thednp commented 1 year ago

@jcorporation I've implemented the feature, however I'm having problems with the Cypress testing suite. The same old fix one crap 2 more pop.

jcorporation commented 1 year ago

@thednp : notify me, as soon it can be tested

thednp commented 1 year ago

@jcorporation the implementation is live, since this morning. I still have some testing scripting to fix.

jcorporation commented 1 year ago

Great, I will test it.

jcorporation commented 1 year ago

Works as intended. Many thanks for this enhancement!