understrap / understrap

Underscores + Bootstrap = Understrap, the renowned open-source WordPress starter theme.
https://understrap.com
GNU General Public License v3.0
3.04k stars 964 forks source link

the `bootstrap` variable as shown in their docs doesn't work in our custom-javascript.js file #1583

Closed bacoords closed 6 months ago

bacoords commented 2 years ago

Dig into why this type of usage from the Bootstrap docs isn't working w/ our build set up

var exampleEl = document.getElementById('example')
var popover = new bootstrap.Popover(exampleEl, options)

https://getbootstrap.com/docs/5.1/components/popovers/#usage

JanikWeb commented 2 years ago

Same just happened to me for tooltips. Glad to see the issue has already been discovered.

IanDelMar commented 2 years ago

Have you tried var popover = new understrap.Popover(exampleEl, options) or switching from "understrap" to "bootstrap" in rollup.config.js? https://github.com/understrap/understrap/blob/dc36e1cb9f59a9935790023220f1a55d7d1bd073/src/build/rollup.config.js#L40

bacoords commented 2 years ago

Thanks @IanDelMar that was helpful. I tested out a few scenarios and this is what I came up with:

Because we're renaming the output to 'understrap' instead of 'bootstrap', if you want to use a Bootstrap constructor OUTSIDE of our build process, like in a separate script or in the site footer or something, it'd have to look like this:

var popoverTriggerList = [].slice.call(document.querySelectorAll('[data-bs-toggle="popover"]'))
var popoverList = popoverTriggerList.map(function (popoverTriggerEl) {
  return new understrap.Popover(popoverTriggerEl)
})

If you're using it inside the custom-javascript.js file and rebuilding the scripts, you don't use the name at all, you actually have to import the class into the custom-javascript file:

import Popover from 'bootstrap/js/dist/popover'

var popoverTriggerList = [].slice.call(document.querySelectorAll('[data-bs-toggle="popover"]'))
var popoverList = popoverTriggerList.map(function (popoverTriggerEl) {
  return new Popover(popoverTriggerEl)
})

Neither of these really matches the Bootstrap documentation, so they're causing confusion all around.

So a few ideas:

IanDelMar commented 2 years ago

should we convert the 'name' variable to 'bootstrap' instead of 'understrap'? Then you can use bootstrap.Popover() in other script files. I see no reason to use 'understrap' here. It's more confusing than helpful.

Understrap bundles more than just Bootstrap JS files, so there is a reason why you might want to name the bundle "understrap".

Concerning the popover issue: popper.js is missing in the bundle, so no matter which fix you choose it won't work. You actually re-compile the dist files... They won't include popper.js automatically as there is no import statement, they expect popper.js being an external script (information you do not provide in the rollup.config.js anyway). Also, by doing this you're bloating the bundle by including the same script parts (eg the Util API) over and over again. For instance, check how many times defineJQueryPlugin is defined.

Consider changing this https://github.com/understrap/understrap/blob/2ab60fea1067c2d00468d20c8851a3fcfcb083f2/src/js/bootstrap.js#L1-L27 to

export { default as Alert } from 'bootstrap/js/src/alert'
export { default as Button } from 'bootstrap/js/src/button'
export { default as Carousel } from 'bootstrap/js/src/carousel'
export { default as Collapse } from 'bootstrap/js/src/collapse'
export { default as Dropdown } from 'bootstrap/js/src/dropdown'
export { default as Modal } from 'bootstrap/js/src/modal'
export { default as Offcanvas } from 'bootstrap/js/src/offcanvas'
export { default as Popover } from 'bootstrap/js/src/popover'
export { default as ScrollSpy } from 'bootstrap/js/src/scrollspy'
export { default as Tab } from 'bootstrap/js/src/tab'
export { default as Toast } from 'bootstrap/js/src/toast'
export { default as Tooltip } from 'bootstrap/js/src/tooltip'

That way you are able to include/exclude a plugin by simply placing or removing a "//" in front of the export statement and include popper.js on the fly if (!) a module imports popper.js (eg if you use scrollspy only which does not need popper.js it won't be included) and include imports only once even if they are imported multiple times.

If you want popper.js to be external you have to add it to external and global in the rollup.config.js. Change this https://github.com/understrap/understrap/blob/2ab60fea1067c2d00468d20c8851a3fcfcb083f2/src/build/rollup.config.js#L28-L30 to

const globals = {
  'jquery': 'jQuery', // Ensure we use jQuery which is always available even in noConflict mode
  '@popperjs/core': 'Popper',
}

and this https://github.com/understrap/understrap/blob/2ab60fea1067c2d00468d20c8851a3fcfcb083f2/src/build/rollup.config.js#L12 to

const external = ['jquery', '@popperjs/core']

Including popper.js and excluding the bloat leads to a file sizes of 209KB unminified and 78KB minified (compared to current sizes of 285KB and 105KB) which is much closer to the original Bootstrap bundle.

IanDelMar commented 2 years ago

Neither of these really matches the Bootstrap documentation, so they're causing confusion all around.

To be fair: the Bootstrap documentation is not written for people bundling Bootstrap like that but for people who a) import Bootstrap modules or b) load dist files in conjunction with custom scripts.

Open this codepen collection for popover examples matching the documentation: https://codepen.io/collection/KpgGre

adambundy commented 1 year ago

I'd love to see some examples of how to work with bootstrap in the custom-javascript.js file as you mentioned @bacoords. I'm trying to do some custom hash-based selection of tabs in page content, and I can get that working, but it breaks BS components outside of these, like the main nav dropdowns. Would it be possible to provide a simple example here? Thanks!

UnaOmnia commented 1 year ago

I'd love to see some examples of how to work with bootstrap in the custom-javascript.js file as you mentioned @bacoords. I'm trying to do some custom hash-based selection of tabs in page content, and I can get that working, but it breaks BS components outside of these, like the main nav dropdowns. Would it be possible to provide a simple example here? Thanks!

Yeah I am literally working on the exact same issue... first .collapse not a function but then realizing that Bootstrap 5 doesn't use that right? it uses the bootstrap variable i think... like you all are saying...

if (window.location.hash != '') { $(window.location.hash).addClass('show'); $("a[href$='" + window.location.hash + "']").removeClass('collapsed').prop('aria-expanded', 'true'); }

Here is a quick work around for that if you are pressed for time but yeah I'd love to get this theme working the way it was intended to...

EDIT: After trying some things I got the minification for CSS and JS to work...

so first i did version 10.11.0 for node which got me to like 6.8 in npm but i still had errors so i went to node 16.19 and that got me npm 8.19 or something and i also ran "npm install --no-audit" to avoid some of the security warns (one for browsersync one for babel and then something about warning depreciated stuff also I skipped past but I heard don't use GULP for Understrap 1.2.2 so i did "npm run watch" and I took some js and css i had called separately and cut paste them into src/js/custom-javacscript.js and src/sass/theme/_child_theme.scss respectively and the minify and js worked but I have not tested the bootstrap variable - I will try to get around to that sometime tonight etc perhaps...

IanDelMar commented 1 year ago

Yes, as stated in package.json you have to use node v16+: https://github.com/understrap/understrap/blob/546b350b62c22d384a563a9c0ad1b77f2331e046/package.json#L34-L36

Using the example from the Bootstrap docs

<!-- Nav tabs -->
<ul class="nav nav-tabs" id="myTab" role="tablist">
  <li class="nav-item" role="presentation">
    <button class="nav-link active" id="home-tab"  type="button" role="tab" aria-controls="home" aria-selected="true">Home</button>
  </li>
  <li class="nav-item" role="presentation">
    <button class="nav-link" id="profile-tab"  type="button" role="tab" aria-controls="profile" aria-selected="false">Profile</button>
  </li>
  <li class="nav-item" role="presentation">
    <button class="nav-link" id="messages-tab" type="button" role="tab" aria-controls="messages" aria-selected="false">Messages</button>
  </li>
  <li class="nav-item" role="presentation">
    <button class="nav-link" id="settings-tab" type="button" role="tab" aria-controls="settings" aria-selected="false">Settings</button>
  </li>
</ul>

<!-- Tab panes -->
<div class="tab-content">
  <div class="tab-pane active" id="home" role="tabpanel" aria-labelledby="home-tab" tabindex="0">Home Tab Content</div>
  <div class="tab-pane" id="profile" role="tabpanel" aria-labelledby="profile-tab" tabindex="0">Profile Tab Content</div>
  <div class="tab-pane" id="messages" role="tabpanel" aria-labelledby="messages-tab" tabindex="0">Messages Tab Content</div>
  <div class="tab-pane" id="settings" role="tabpanel" aria-labelledby="settings-tab" tabindex="0">Settings Tab Content</div>
</div>

this is what your custom-javascript.js could look like:

import Tab from 'bootstrap/js/src/tab';

const selectTabByHash = () => {
    let triggerEl;
    const hash = window.location.hash;

    if (hash) {
        triggerEl = document.querySelector(`#myTab button[data-bs-target="${hash}"]`);
        if (triggerEl) {
            Tab.getOrCreateInstance(triggerEl).show();
        }
    }
}
selectTabByHash();
window.addEventListener( 'hashchange', selectTabByHash)

It's even sufficient to use

if (triggerEl) {
    triggerEl.click();
}
IanDelMar commented 1 year ago

@bacoords I suggest to add instructions such as in https://github.com/understrap/understrap/issues/1583#issuecomment-1022444176 to the docs and close this issue.

UnaOmnia commented 1 year ago

NICE!

Looks like what was missing was:

import Tab from 'bootstrap/js/src/tab';

to get it all started - I am starting a fresh install soon here so I will see how it goes...

all of a sudden I am having issues with CSS compile not liking ":"

IanDelMar commented 1 year ago

all of a sudden I am having issues with CSS compile not liking ":"

Feel free to open a new issue if you'd like help solving the problem with CSS compilation.

mattneal-stafflink commented 1 year ago

I was having this issue, when I was trying to setup Tooltips. I just replace the bootstrap class to understrap.

bacoords commented 6 months ago

Closing this now that we've added a note to the documentation.