Please add support for Bootstrap 5

[cryptomilk]

The Bootstrap 4 renderer works with some css fixes, but please add support for Bootstrap 5.

Thanks for considering.

[henning-unicode]

I've started working on making the renderer Bootstrap 5 compatible. Not sure how soon it will be done.

[henning-unicode]

@cryptomilk It would be great if you could test my PR for Bootstrap 5 support: #9

[cryptomilk]

 Gemfile                                             |  3 ++-
 Gemfile.lock                                        | 14 ++++++++++----
 app/assets/stylesheets/application.scss.erb         | 12 ++++++++----
 lib/simple_navigation_bootstrap/custom_bootstrap.rb |  6 ++----
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/Gemfile b/Gemfile
index 9ec9292..df3a96f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -25,7 +25,8 @@ gem 'simple_form'
 # https://github.com/codeplant/simple-navigation/issues/231
 gem 'ostruct'
 gem 'simple-navigation'
-gem 'simple_navigation_bootstrap'
+# TESTING: Bootstrap 5 support PR - https://github.com/jbox-web/simple_navigation_bootstrap/pull/9
+gem 'simple_navigation_bootstrap', github: 'henning-unicode/simple_navigation_bootstrap', branch: 'feature/bootstrap5'
 
 # Use ActiveModel has_secure_password
 gem 'bcrypt'

diff --git a/app/assets/stylesheets/application.scss.erb b/app/assets/stylesheets/application.scss.erb
index 296229c..4086480 100644
--- a/app/assets/stylesheets/application.scss.erb
+++ b/app/assets/stylesheets/application.scss.erb
@@ -276,15 +276,19 @@ div.topo {
     background-color: #343a40 !important;
 }
 
+// TESTING: Commented out while testing Bootstrap 5 support from
+// https://github.com/jbox-web/simple_navigation_bootstrap/pull/9
+// If navigation looks correct, this can be removed permanently.
+//
 // Fix navbar navigation items padding in collapsed state
 // NOTE: We're using Bootstrap 5, but simple_navigation_bootstrap 1.3.0 only supports Bootstrap 4.
 // In Bootstrap 5, navbar items in the collapsed state (mobile menu) need explicit horizontal padding
 // to prevent them from being flush against the left edge. Bootstrap 4 had this built-in,
 // but Bootstrap 5 removed it, so we add it back here.
-.navbar-collapse .navbar-nav .nav-link {
-    padding-left: 2rem;
-    padding-right: 2rem;
-}
+// .navbar-collapse .navbar-nav .nav-link {
+//     padding-left: 2rem;
+//     padding-right: 2rem;
+// }
 
 // Bootstrap 5: Apply form-select styling to select elements
 // In Bootstrap 5, select elements need the form-select class to show the dropdown arrow indicator.
diff --git a/lib/simple_navigation_bootstrap/custom_bootstrap.rb b/lib/simple_navigation_bootstrap/custom_bootstrap.rb
index bb927bd..90d14e0 100644
--- a/lib/simple_navigation_bootstrap/custom_bootstrap.rb
+++ b/lib/simple_navigation_bootstrap/custom_bootstrap.rb
@@ -10,11 +10,9 @@ module SimpleNavigationBootstrap
     end
 
     # Returns Bootstrap version for simple_navigation_bootstrap gem
-    # NOTE: We're actually using Bootstrap 5, but simple_navigation_bootstrap 1.3.0
-    # only supports Bootstrap 4. We return 4 here and handle Bootstrap 5 compatibility
-    # differences with custom CSS (see app/javascript/stylesheets/application.scss)
+    # TESTING: Using Bootstrap 5 support from PR https://github.com/jbox-web/simple_navigation_bootstrap/pull/9
     def bootstrap_version
-      4
+      5
     end
 
     def container_class(_level)

It looks like those padding fixes are still needed. Else the the items hit the left side of the menu.

[henning-unicode]

@cryptomilk thanks for testing my PR! With the application I am testing it I don't need the additional CSS you need. The responsive menu looks fine. Did you check if you are using the correct html structure for the navigation?

In our application it looks like this (haml):

  .navbar.navbar-expand-lg.bg-body-tertiary
    .container-fluid
      %a.navbar-brand
        Brand
      %button.navbar-toggler(type="button" data-bs-toggle="collapse" data-bs-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation")
        %span.navbar-toggler-icon
      #navbarSupportedContent.navbar-collapse.collapse
        = render_navigation expand_all: true, renderer: :bootstrap5

[n-rodriguez]

@henning-unicode let me know when you're done, I'll review your PR. Thank you!

[cryptomilk]

Thanks, I was able to fix it!

[cryptomilk]

I think I found a bug.

In rendered_item5.rb, the simple_link method copies options[:class] to the link element:

link_options[:class] = [options[:class], link_class].flatten.compact.join(" ")

However, options[:class] contains classes meant for the <li> element (like nav-item), not the <a> element.

Current output:

<li class="nav-item">
  <a class="nav-item nav-link" href="/path">Link Text</a>
</li>

Expected (Bootstrap 5 structure):

  <li class="nav-item">
    <a class="nav-link" href="/path">Link Text</a>
  </li>

This breaks layouts when custom classes are added to navigation items - they end up on the <a> element causing CSS issues like incorrect text wrapping in navbar items.

[cryptomilk]

@henning-unicode ^^ ;-)

[henning-unicode]

@cryptomilk I finally came around to fix the bug you found, thank you!

@n-rodriguez It is ready for review now!

[henning-unicode]

@cryptomilk ah now I remember why I used options[:class]: bootstrap 5 expects the active class on the a and it used to be on the li.

[n-rodriguez]

Bootstrap 5 support has landed on master (commit 13c020a, released as 1.4.0). It ships a dedicated :bootstrap5 renderer (bootstrap5.rb / rendered_item5.rb) with dropdown menus, dividers, headers, split toggles, nested submenus and parity specs.

Note on the options[:class] bug @cryptomilk reported: our renderer keeps nav-item on the <li> and nav-link/active on the <a> (BS5 puts the active state on the link), so custom item classes no longer leak onto the <a>.

Thanks @henning-unicode and @cryptomilk for kicking this off and testing. Closing as implemented — please open a new issue if anything is off on your side.