thednp / bootstrap.native

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

Bootstrap 5 version available, feedback needed #400

Closed thednp closed 3 years ago

thednp commented 3 years ago

I've added a Bootstrap 5 source to our code, there are some minor changes to DATA API, for instance data-toggle="tooltip" => data-bs-toggle="tooltip", and other markup changes not interfering with our code base.

I'm expecting the usual:

BSN for Bootstrap 5 changes (compared to V4):

Thanks and have fun testing.

cvkmohan commented 3 years ago

Thanks for this. I will put effort to contribute. To admit honestly, your responses created a chain of thoughts - and - I am confident that they will result in my active participation in open source.

newyhj commented 3 years ago

good ieda

thednp commented 3 years ago

I've uploaded the latest code. I hope you guys have a look at V5 and share your thoughts.

@cvaize @midzer @fmasa AND anyone is welcomed to join :)

cvaize commented 3 years ago

@thednp Thank you for your work! I need more time to check and compare everything. For now, I will create 1 issue. It is associated with a modal window and data-bs-backdoor= "static".

cvaize commented 3 years ago

At the moment, "bootstrap.native" is 86% smaller, and I am happy about it. image

thednp commented 3 years ago

@cvaize hey there. I'm looking into ways to make it even smaller.

Other contributors, @xHeinrich @picasticks @jakubboucek @TheCodeExorcist @stepheny and anyone else, help is still needed :)

thednp commented 3 years ago

@cvaize there you go, BSN for Bootstrap 5, same size minimized, more features added, this should be even more awesome.

Everyone else, enjoy the testing ;)

midzer commented 3 years ago

Sorry for not responding for a while.

In the last months I've migrated my only BS 5 site to original JS plugins due lack of BSN support. TBH I don't care so much about reduced file size. What is more important to me is how I can use individual components (Carousel, Collapse, etc) of each library without having the need to bundle them separatelty (or via a loader).

This is what I simply do in my BS 5 project webpack main.js file:

import Button from 'bootstrap/js/dist/button';
import Carousel from 'bootstrap/js/dist/carousel';
import Collapse from 'bootstrap/js/dist/collapse';
import Dropdown from 'bootstrap/js/dist/dropdown';
import Tab from 'bootstrap/js/dist/tab';

^So I can grab only JS components I need.

How would I do it with BSN5?

thednp commented 3 years ago
import Button from 'bootstrap-native/src/components-v5/button-native';
import Carousel from 'bootstrap-native/src/components-v5/carousel-native';
import Collapse from 'bootstrap-native/src/components-v5/collapse-native';
import Dropdown from 'bootstrap-native/src/components-v5/dropdown-native';
import Tab from 'bootstrap-native/src/components-v5/tab-native';

init your way

EDIT: later when Bootstrap 5 is stable you use bootstrap-native/src/components/

thednp commented 3 years ago

@midzer the latest version is on npm, you can give it a try and let us know what's up :)

apiontek commented 3 years ago

Should Dropdown keyboard navigation work? v5.html says yes, but it's failing for me.

Version 3.0.14f

With the default official v5 BS js, with popperjs, it works - loaded in my app.js like so:

import Collapse from "bootstrap/js/dist/collapse";
import Dropdown from "bootstrap/js/dist/dropdown";

This gives me a dropdown that highlights items with up/down arrow keys.

But if I uninstall popper, install bootstrap.native, and replace my app.js lines like so:

import BSN from "bootstrap.native/src/index-v5";

Then the dropdown opens and closes, and if I open it with keyboard, ESC works to close it, but up/down arrows do nothing.

thednp commented 3 years ago

Thanks for dropping by. The up + down should work when you focus on the dropdown menu. Perhaps not as perfectly identical to the original.

thednp commented 3 years ago

@apiontek can you confirm that for me?

apiontek commented 3 years ago

No, it doesn't work for me. Not sure if I'm supposed to do anything different but I can say that, with:

import BSN from "bootstrap.native/src/index-v5";

as the only thing I'm doing to load BSN, a Dropdown menu works, but up/down doesn't.

To be more specific, on page load, when I start pressing Tab on my keyboard, it cycles through elements on the page, including the dropdown menu. I press Enter to activate the dropdown menu and it appears as desired, but then following that with Down or Up does nothing.

Is there an extra step to "focus on the dropdown menu?"

thednp commented 3 years ago

Another tab key?

apiontek commented 3 years ago

OK, yes, I can confirm that another Tab after the Enter to open the menu brings me into the menu, and then up/down work.

thednp commented 3 years ago

You can also hit SPACE bar to open the dropdown ;)

apiontek commented 3 years ago

Space bar doesn't work for me to open the dropdown :'(

thednp commented 3 years ago

Works for me on Chrome browsers, if you're on a MAC, I don't.....

apiontek commented 3 years ago

I'm testing in Chrome, Firefox, and Chromium based MS Edge, all on Windows. It's fine, just reporting feedback!

newyhj commented 3 years ago

bootstrap v5 beta3 added offcanvas component.

thednp commented 3 years ago

Thanks, will have a look when I get the time.

thednp commented 3 years ago

@newyhj Offcanvass added, please check and test the latest master. After some revisit I will also add to npm.

@cvaize you can also have a look at the V5 new codebase.

newyhj commented 3 years ago

@thednp can't use bootstrap-native-v5.js or bootstrap-native-v5.min.js merge and minifier with other js files to single file. maybe there is a problem with the native js file code. i'm sure other js no problem,please check it.

thednp commented 3 years ago

The code is in ES6+ flavor in those files, just like the original Bootstrap 5. Maybe that's why.

If that is the case, you may need to configure an ES5 compiler.

newyhj commented 3 years ago

I've solved the merge problem

dyanakiev commented 3 years ago

Hello, i get error when i require either the compiled lib or the src for v5

image

  function hasClass(element, classNAME) {
    return element.classList.contains(classNAME); // this is line 1942
  }

Line 58 in the bootstrap-native-v5.js, theres something in my html thats triggering the error but i dont know what exactly if i try it on an empty page its fine.

dyanakiev commented 3 years ago

Something that i noticed in bootstrap's js, when i have dropdown if i have data-bs-display this gets applied to the dropdown-menu as data-bs-popper="static" when opened, with bootstrap-native-v5 this doesnt work

data-bs-toggle="dropdown" data-bs-display="static"
thednp commented 3 years ago

@dyanakiev thanks for dropping by.

First, we're now packaging BSN in ES6+ flavor, perhaps that's why your require doesn't work? I don't exactly know what's in your setup. So I took a quick guess.

Secondly as for the new display option for Dropdown, it's not 100% tested in all cases, you can provide a sample code we can have a look.

Thanks

dyanakiev commented 3 years ago

@thednp Im requiring the package that way:

window.BSN = require('bootstrap.native/dist/bootstrap-native-v5')
or 
window.BSN = require('bootstrap.native/src/index-v5')

With both i get the error. -- Edit about the exception i had misplaced data-bs-toggle="dropdown" in the html on elements that are not dropdowns, example <a href="#" data-bs-toggle="dropdown" class="text-muted"><i class="fa fa-redo"></i></a> im still not sure if this should cause any issues but it does, after correcting the html (removing the undeeded data-bs-toggle=dropdown the error was gone.

Here is the sample for the dropdown that doesnt add the display static.

<div class="menu-item dropdown">
                <a href="#" data-bs-toggle="dropdown" data-bs-display="static" class="menu-link">
                    test
                </a>
                <div class="dropdown-menu dropdown-menu-end me-lg-3">
                    <a class="dropdown-item d-flex align-items-center" href="#">test</a>
                </div>
</div>

Thanks

thednp commented 3 years ago

Everybody!

Bootstrap 5 is now stable, our library has switched V5 as the default package bootstrap-native.js, V4 is now bootstrap-native-v4.js.

swrobel commented 3 years ago

@thednp I don't want to sound ungrateful because this is amazing & much appreciated, but I wish you'd made a major version bump to communicate that we needed to change our imports for Bootstrap 4 with this release. The patchlevel bump shouldn't necessarily indicate any breaking changes, so I was surprised when I discovered that the js was no longer working on my site.

thednp commented 3 years ago

Thanks for the suggestion.

pjcarly commented 3 years ago

I second @swrobel, a major version bump would have been the correct way to go. Semver makes our lives easier. I had the same problem, the dropdown component stopped working when upgrading to 3.0.15, still works on 3.0.14.

This was the error I was having Cannot read property 'map' of undefined

Don't take me wrong, I really appreciate your hard work and love the project.

thednp commented 3 years ago

Do you guys consider a major version bump for a github releases as well or just npm/cdn?

pjcarly commented 3 years ago

Semver says:

<major>.<minor>.<patch> MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards compatible manner, and PATCH version when you make backwards compatible bug fixes.

Regardless of the place where the release is posted. semver.org

thednp commented 3 years ago

@pjcarly & @swrobel guys, I found some time to take care of this, I'm sorry for the confusion I've caused, I've updated my entire JavaScript codebase on github in the last few months or so, and completely forgot about semver, I hope I didn't cause too much trouble, though I've put much effort into this task.

As usual, please update your stuff, test and provide feedback.

Cheers :)

thednp commented 3 years ago

We are now fully into Bootstrap 5 and we're most likely OK to close this one down.

Thanks for everyone's feedback.

pjcarly commented 3 years ago

Luckily for me it was a quick, edit package.json back to previous version, run yarn, commit, and let the CI/CD do the deploy. A few minutes work.

Still @thednp, thanks for your hard work.

cvkmohan commented 3 years ago

Thanks for the hard work @thednp. I work with phoenix LiveView - so got into some issues in making the Toast work. Otherwise, both documentation and implementation are extremely user friendly. I was working with Bootstrap 5 directly from the time I came to know of your library. So, did not get any issues. Thanks and keep up the work.

swrobel commented 3 years ago

@thednp thanks for bumping the major version. This is a step in the right direction. I think to fully rectify this, you should push a 3.0.16 version that is basically a copy of 3.0.14, as 3.0.15 is the version that breaks the previous default behavior, and that is still the latest in the 3.x series. For example, my previous package.json line was "bootstrap.native": "^3.0.13" which I then changed to "bootstrap.native": "3.0.14" to avoid having it grab 3.0.15, if that makes sense. I realize this is a time-suck of a task, so no pressure to do it! That's just my two cents on how to really fix the whole SemVer issue.

thednp commented 3 years ago

@swrobel I wish this kind of feedback would have come earlier before we switched V4 official to V5 official. But hey can't keep track of a million things :)

dehy commented 3 years ago

@swrobel I wish this kind of feedback would have come earlier before we switched V4 official to V5 official. But hey can't keep track of a million things :)

It breaks semver. That's unusual for a patch version to introduce breaking changes, given the inner-working of npm/yarn versioning notation.