Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootstrap 5 version available, feedback needed #400

Closed
33 tasks done
thednp opened this issue Jan 12, 2021 · 43 comments
Closed
33 tasks done

Bootstrap 5 version available, feedback needed #400

thednp opened this issue Jan 12, 2021 · 43 comments
Labels
enhancement potential improvement help wanted need your suggestion

Comments

@thednp
Copy link
Owner

thednp commented Jan 12, 2021

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:

  • bug reports, also documentation related
  • suggestions on improvements
  • any and all feedback

BSN for Bootstrap 5 changes (compared to V4):

  • ES6+ class declarations for components, the implementation keeps methods/properties private for increased security; all instances expose only public methods and nothing else; we use a cool prototype pattern to save as much space as possible
  • Rework all shorter-js utilities we use on BSN for ES6+
  • Carousel methods changed: added next() and prev() methods, while the slideTo() have been renamed to to()
  • Dropdown automatic positioning on scroll/resize, a feature similar to Tooltip/Popover and is implemented in a way that it uses the Bootstrap 5 classes (Eg: dropup, dropend) to show the dropdown menu in the viewport, however dropdown nesting is no longer supported
  • Dropdown inconsistencies with dismissing related event handlers have been eliminated
  • ScrollSpy performance optimizations: now the component will update the targets only when HTMLCollection is updated or the offsets change due to window resize
  • ScrollSpy ability to activate last item at maximum scrollHeight even when not in "scroll range" Need to configure ScrollSpy #353, initially believed to be impossible
  • ScrollSpy has a much better event handling capability; while you can have multiple instances, all event listeners are grouped and properly handled on both initialization and the .dispose() call
  • Popover & Tooltip rework styleTip utility to respect the boundaries of the container, solving issues with miss-aligned tooltips and popovers exceeding the right side of the container
  • Popover & Tooltip have new methods: .enable(), .disable() and .toggleEnabled()
  • Popover & Tooltip have new method: .update(), replacing a previously featured private method
  • Popover & Tooltip now support aria-describedby
  • Popover & Tooltip no longer null their generated popover / tooltip on .hide() calls, a much needed performance improvement; to update their contents you must re-initialize them with fresh new options values / contents
  • Popover & Tooltip now always use a template, saves a lot of space
  • Popover & Tooltip now support more container and target positions
  • Popover & Tooltip now support mousemove event for Media elements like <svg>, <img>, <video> (demo updated), for instance a large image initialized with Tooltip/Popover would display its tip outside the viewport even with the automatic repositioning OR transparent images (both SVG and PNG) might cause issues with displaying a tip; the "follow the cursor" feature currently only works with hover triggering option
  • Popover & Tooltip automatic repositioning on scroll, previously not supported, now we're on par with original plugins
  • Collapse will block any click when a sibling collapse part of an accordion is in isAnimating state
  • Popover & Tooltip sanitize function option
  • Modal can do the zoom in transition when you click a static modal
  • Modal no longer supports content option and .setContent() instance method
  • Code cleanup (replace old codebase with modern code, EG: indexOf => Array.prototype.incudes)
  • Applying fixes (Nested links not clickable in collapse toggle #398 Fix issue where null collapse throws a type error #401 No response of the modal window to a click on a static background (data-bs-backdrop="static", data-backdrop="static"). #402 done) and any other problem reported during the next period until Bootstrap 5 is stable
  • Added a normalizeOptions utility to save even more space
  • Reworked the BSN.initCallback()
  • Reworked Button component to support state toggle and removed all its previous functionality
  • Documentation and WIKI updates
  • Added ESLint before the build, update the code.
  • Added Offcanvas component
  • Added BaseComponent to handle with most common component instances code, hopefully helping with overall size;
  • Modal, Offcanvas, Collapse, Toast and Alert components will work closer to the original plugins in the sense that <div class="collapse"> is this.element of the Collapse instance as well as the element with options DATA API attributes;
  • Added a set of utilities for Modal and Offcanvas to handle overlay, if you open a new modal/offcanvas instance while also a modal/canvas is open, the current instance will close;
  • improve performance, security and simplicity

Thanks and have fun testing.

@thednp thednp added the help wanted need your suggestion label Jan 12, 2021
@cvkmohan
Copy link

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
Copy link

newyhj commented Jan 24, 2021

good ieda

@thednp
Copy link
Owner Author

thednp commented Jan 28, 2021

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
Copy link
Contributor

cvaize commented Jan 29, 2021

@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
Copy link
Contributor

cvaize commented Jan 29, 2021

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

@thednp
Copy link
Owner Author

thednp commented Jan 29, 2021

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

  • I'm looking for a way to change the class Alert declaration to Alert.prototype compile options, something to store the prototype into a variable like alertProto = Alert.proto, this takes a huge chunk of space
  • I'm also considering creating a BS specific string library for all those data-bs-toggle attributes
  • anything we can do to improve

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

@thednp
Copy link
Owner Author

thednp commented Feb 1, 2021

@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 ;)

thednp added a commit that referenced this issue Feb 1, 2021
* reworked `emulateTransitionEnd` `getTransitionDuration` and other related utilities
* added a new tool to `normalizeOptions` saves a ton of space
* ES6 class declarations in a custom scope
* re-added `shorter-js` classList shorthands
* other changes are already explained in #400
thednp added a commit that referenced this issue Feb 1, 2021
* reworked `emulateTransitionEnd` `getTransitionDuration` and other related utilities
* added a new tool to `normalizeOptions` saves a ton of space
* ES6 class declarations in a custom scope
* re-added `shorter-js` classList shorthands
* other changes are already explained in #400
@midzer
Copy link
Contributor

midzer commented Feb 1, 2021

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
Copy link
Owner Author

thednp commented Feb 1, 2021

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
Copy link
Owner Author

thednp commented Feb 4, 2021

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

@apiontek
Copy link

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
Copy link
Owner Author

thednp commented Feb 25, 2021

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
Copy link
Owner Author

thednp commented Feb 26, 2021

@apiontek can you confirm that for me?

@apiontek
Copy link

apiontek commented Mar 5, 2021

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
Copy link
Owner Author

thednp commented Mar 5, 2021

Another tab key?

@apiontek
Copy link

apiontek commented Mar 5, 2021

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
Copy link
Owner Author

thednp commented Mar 5, 2021

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

@apiontek
Copy link

apiontek commented Mar 5, 2021

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

@thednp
Copy link
Owner Author

thednp commented Mar 5, 2021

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

@apiontek
Copy link

apiontek commented Mar 5, 2021

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

@newyhj
Copy link

newyhj commented Mar 24, 2021

bootstrap v5 beta3 added offcanvas component.

@thednp
Copy link
Owner Author

thednp commented Mar 24, 2021

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

@thednp
Copy link
Owner Author

thednp commented Apr 12, 2021

@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
Copy link

newyhj commented Apr 13, 2021

@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
Copy link
Owner Author

thednp commented Apr 13, 2021

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.

@dyanakiev
Copy link

dyanakiev commented Apr 16, 2021

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
Copy link

dyanakiev commented Apr 16, 2021

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
Copy link
Owner Author

thednp commented Apr 16, 2021

@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
Copy link

dyanakiev commented Apr 16, 2021

@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
Copy link
Owner Author

thednp commented May 7, 2021

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
Copy link

swrobel commented Jun 5, 2021

@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
Copy link
Owner Author

thednp commented Jun 5, 2021

Thanks for the suggestion.

@pjcarly
Copy link

pjcarly commented Jun 6, 2021

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
Copy link
Owner Author

thednp commented Jun 7, 2021

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

@pjcarly
Copy link

pjcarly commented Jun 7, 2021

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 added a commit that referenced this issue Jun 9, 2021
thednp added a commit that referenced this issue Jun 9, 2021
@thednp
Copy link
Owner Author

thednp commented Jun 9, 2021

@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
Copy link
Owner Author

thednp commented Jun 9, 2021

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

Thanks for everyone's feedback.

@pjcarly
Copy link

pjcarly commented Jun 9, 2021

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
Copy link

cvkmohan commented Jun 9, 2021

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
Copy link

swrobel commented Jun 10, 2021

@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
Copy link
Owner Author

thednp commented Jun 10, 2021

@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
Copy link

dehy commented Jun 16, 2021

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement potential improvement help wanted need your suggestion
Projects
None yet
Development

No branches or pull requests

10 participants