Dropdown in Navbar

[supergibbs]

Issue description

• components: NavDropdown
• reactstrap version #5.0.0-alpha.3
• bootstrap version #4.0.0-beta

What is happening?

In a navbar, the Dropdown still uses popper to position it

What should be happening?

It should just use Bootstrap's CSS when inside a Navbar
Bootstrap disables popper in a navbar: https://github.com/twbs/bootstrap/blob/v4-dev/js/src/dropdown.js#L146
Example here: https://getbootstrap.com/docs/4.0/components/navbar/ (especially noticeable when navbar is collapsed)

[maazadeeb]

@supergibbs I'd like to take this up. I think I understand the issue, but I'd like to get it clarified before I begin trying to fix this:

Is it that Dropdown is currently agnostic of the fact that whether it is in a NavDropdown or not? And Dropdown's render returns a react-popper Manager always?

Does it mean I need to change this to use Tag when it's in a NavDropdown?

I'm new to this project and I could've understood it completely wrong. Please feel free to correct me and guide me in the right direction.

[supergibbs]

Yes, the current Dropdown always uses popper but it should only use it when not in a Navbar.

Note, this is different than just modifying <NavDropdown /> (or <Dropdown nav /> once #636 is merged). It needs to know that it's in a Navbar, not any Nav, possibly with a new prop if needed.

[maazadeeb]

Alright. I'll start working on it. @TheSharpieOne, could you please add the in-progress tag for this, since I'm working on it?

[maazadeeb]

@supergibbs I've been looking around the code and seeing how I can fix this. Since Navbar is a stateless component, I've to first make it stateful and then use context to pass down a isInNavbar prop to its children. Then, I'll have to use this isInNavbar flag in Dropdown as well as its children in order to remove any dependencies on react-popper.

Is this the best possible solution or you have other suggestions?

[TheSharpieOne]

Probably just best to add a navbar prop to the dropdown and trigger logic off if that.

[supergibbs]

Hey @maaz93, how's it going? Just a heads up that #636 was merged in which may affect your PR.

If you aren't working on this anymore, let me know and I can do it. Thanks!

[maazadeeb]

Hi @supergibbs ! I haven't found time to do this yet. Please go ahead and do it!

[supergibbs]

@TheSharpieOne I did this two different ways:

#693 One is a very simple prop on DropdownMenu to ignore Popper.

#692 The other is a little more complex, using context to remove Popper and maybe a little more performant?

Let me know which approach you like better.

[TheSharpieOne]

I like the context approach (even though using context is typically not the best.... but we are already using it here) as it may lend to having the Navbar itself set the context automatically in the future.