Skip to content

Only enqueue jQuery if Bootstrap version = 4#1760

Open
bbbenji wants to merge 1 commit intounderstrap:developfrom
bbbenji:patch-2
Open

Only enqueue jQuery if Bootstrap version = 4#1760
bbbenji wants to merge 1 commit intounderstrap:developfrom
bbbenji:patch-2

Conversation

@bbbenji
Copy link
Copy Markdown

@bbbenji bbbenji commented May 27, 2022

Description

Only enqueue jQuery if Bootstrap version = 4

Motivation and Context

Bootstrap 5 no longer requires jQuery. This change only enqueues jQuery, if Bootstrap 4 is selected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I pulled my branch from develop.
  • I am submitting my pull request to develop.
  • I have resolved any conflicts merging this pull request would create.
  • I have checked that there aren't other open Pull Requests for the same update/change.
  • My code follows the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • (Optional) My change requires a change to the documentation.
  • (Optional) I have updated the documentation accordingly.
  • (Optional) My change requires a change to the translations.
  • (Optional) I have updated the translations accordingly.
  • composer cs:check has passed locally.
  • composer lint:php has passed locally.
  • I have read the CONTRIBUTING document.

@bacoords
Copy link
Copy Markdown
Member

I haven't tested this out yet, but in theory this should be fine. Other plugins and child themes can still load jQuery if they need it- the starter child theme does...

I'll run some tests, but open to discussion from anyone who sees any issues here, as this technically would be a change for sites that are live with BS5 and the parent theme active.

@IanDelMar
Copy link
Copy Markdown
Contributor

I am in favor of this change. However, this is a breaking change and I suggest to wait for other breaking changes in order to minimize the number of breaking changes.

@IanDelMar
Copy link
Copy Markdown
Contributor

The hero slider uses jQuery. I made a PR removing jQuery. See #1840

@bacoords
Copy link
Copy Markdown
Member

bacoords commented Aug 9, 2022

The more I think about this, the more I think it's a good thing:

  • Understrap still defaults to BS4, so most users won't be affected.
  • The child starter theme has separately enqueued jQuery anyway, so users of the child starter theme should be fine (though I plan to remove it there once this version is released since the child starter now only supports BS5)
  • It doesn't 'remove' jQuery as much as it doesn't 'ask for it', meaning any plugin that needs it will have enabled it. If a user was adding custom JS somehow, then they should have the ability to enqueue it

Still leaving this open for comment.

@bacoords bacoords added this to the 1.3.0 milestone Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants