Skip to content
This repository was archived by the owner on Apr 19, 2025. It is now read-only.

Conversation

@JulioHD
Copy link

@JulioHD JulioHD commented Mar 4, 2022

Hi there

I came across some issues using collapse within a navbar, with this following issues should be fixed:

  • Animation triggers when opening a page using a navbar with a collapse
  • Animation triggers when the screen gets smaller and the collapse starts to hide (and other way around)
  • Toggler button for collapse is not being focused when clicked
  • Toggler button can be toggled mid animation (not the case with bootstrap's js)

The collapse no longer removes and re-adds it's content to the dom, instead it uses bootstrap's "d-none" class to hide it. I do that so bootstrap's "navbar-expand-xx" class can show the collapse content again, when a certain screen size is reached. We no longer need to check for that and it won't animate for a second.

Example code:

<Navbar expand="md">
  <NavbarBrand href="#">Test</NavbarBrand>
  <NavbarToggler id="exampleNavToggler"/>
  <Collapse toggler="#exampleNavToggler" navbar>
    <Nav class="ms-auto" navbar>
      <NavItem>
        <NavLink href="#">Test</NavLink>
      </NavItem>
    </Nav>
  </Collapse>
</Navbar>

Unfortunately I couldn't find a solution how to prevent isOpen from changing, when it's accessed directly.

JulioHD added 5 commits March 4, 2022 00:06
The component was having a short animation when opening a page with a navbar using it.
Also the transition between screen sizes was also animated (when using expand attribute).
This also fixes the hardcoded screen sizes and the corresponding pixels.
The component will no longer be removed and readded to the dom, when it's open state changes.
Animations work again and the collapse can't be triggered multiple times while still in transition, unless the isOpen attribute is directly manipulated.
The behaviour now mirrors bootstrap's js solution.
With my previous commit it was possible to overwrite onEntering for example, causing transitioning not to be set.
Also I fixed some formatting
- "Cannot use import statement outside a module" shouldn't happen any longer
@fkrauthan
Copy link

Any news on getting this merged? Especially that when using Collapse in a navbar that on desktop it has a fade in animation is really annoying.

@JulioHD
Copy link
Author

JulioHD commented Mar 14, 2023

Any news on getting this merged? Especially that when using Collapse in a navbar that on desktop it has a fade in animation is really annoying.

Maintainer seems to be inactive again, never heard from him on this issue. This pull request is a bit unclean anyways, I mistakenly fixed another issue in the branch for my personal project, so I get why this isn't being accepted. I would correct that, if needed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants