-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add Bootstrap 5 support #798
Conversation
jstark518
commented
Oct 21, 2021
•
edited
Loading
edited
- Fixes Any plans for BS5 support? #777 - Builds on PR791: Add support for bootstrap 5 while keeping support for 4
- Changed test to use Chrome
- Changed test to look for .btn-close when using BS5, .close when using BS4.
- Added another test config, test both BS4 and BS5.
- Change to @popperjs/core from popper, see PR746.
Change default date value in test to ISO 8601. Fix a problem where value is set to null in select in bootbox.prompt if it's not defined, this was causing the select to be empty and test fails. Move to newer version of popper.
Update test logic to test BS5 close buttons Add Test config for both BS5 and BS4
@@ -789,7 +789,9 @@ | |||
}); | |||
|
|||
// safe to set a select's value as per a normal input | |||
input.val(options.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#798 This was causing problems with the existing test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, if this is for BS5 and BS4 then we should - imho - have test matrix for each of those - to make sure that all works as expected. |
I don't know enough about Karma to know how to do that, I just made two test configs and each runs all test on both BS4 & BS5. |
@@ -6,7 +6,7 @@ | |||
"test": "tests" | |||
}, | |||
"scripts": { | |||
"test": "./node_modules/.bin/karma start" | |||
"test": "./node_modules/.bin/karma start karma.conf.js && ./node_modules/.bin/karma start karma-bs4.conf.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry about that - I remember that there was some TravisCI / GitHub Actions to run those tests on this repository - Seems like I didn't remember that correctly :(
Is this project abandoned? |
Depends on what you mean. I haven't seen @makeusabrew around for a while, but @tarlepp and I do what we can, when we can, to address issues as they come up. If your expectation was for your pull-request to be immediately either approved or rejected, then I guess it's up to you how you view the status of this project. If you want a simple answer: no, the project is not abandoned, we just don't move very quickly. |
Ah ok ty |
Good Stuff. Just waiting to get this merged and have bootstrap 5 support. |
Thanks for this PR! One thing I found is when modal-header is not there, the X / close button still appears but within the modal-body, and is on the left side. Looks out of place. I'll let you know if I find anything else popping up while I play with it for a bit. |
Thank you, I'll fix that Monday. |
@Yohn Fixed |
Would like to see this PR merged. I have a site that uses bootbox heavily and I am updating it to bootstrap 5. bootbox and a few other packages are holding me back. If there was a replacement to bootbox I would use it. I spent a day trying to hammer SweetAlert2 in only to give up. Nothing out there that I can find has the dialog functionality. |
@bobrundle I wouldn't hold out hope it gets merged, do you use scss? |
@jstark518 I would not be so pessimistic. Your BS5 support will be merged eventually. Things move slow here as they often when free labor is at work. BS5 support is a major improvement and needs to be carefully evaluated. The site I am working on does not use scss but I have integrated your changes into it as I await the official bootbox BS5 support. |
Can't wait for Bootstrap 5 support, this is my workaround written in Sass based on Bootstrap 5 .btn-close, changed the color to transparant to prevent a double cross icon. Hope this temporary fix is helpfull:
|
I did the same exact thing |
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.4 to 1.14.8. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](follow-redirects/follow-redirects@v1.14.4...v1.14.8) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [log4js](https://github.com/log4js-node/log4js-node) from 6.3.0 to 6.4.1. - [Release notes](https://github.com/log4js-node/log4js-node/releases) - [Changelog](https://github.com/log4js-node/log4js-node/blob/master/CHANGELOG.md) - [Commits](log4js-node/log4js-node@v6.3.0...v6.4.1) --- updated-dependencies: - dependency-name: log4js dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
https://github.com/erobertson42/bootstrap-modbox Yes, this is my library. It is vanilla JavaScript for Bootstrap 5, no dependencies, and uses standard Bootstrap classes/styling with no custom CSS. So if you're already using a theme or your own Bootstrap Sass modifications, they should automatically "just work". I was on the fence about replying with this, as I wasn't sure if it was bad form or not, though that wasn't my intention. Mods, if you disapprove of me posting this here, I understand if you remove it - apologies and no hard feelings. I'm not really out for advertising my library, I've never even posted about it anywhere before this, just trying to help with a possible alternative. I also wanted to mention that while the library has no relation to bootbox, I did model the docs site layout after bootbox's (imitation -> flattery 🙂 ). |
Ha. Glad to know that the docs were useful. Did you hard-code everything, as I did, or are you using Jekyll, Gatsby, or similar to build them? |
I originally considered automating it, but I had (and still have) no experience with those tools, so it I figured it was faster/simpler to keep it old school and hardcode it all. It's funny, one of my first ideas for naming the library was actually "bootbox", before I did a search and realized it was already taken. |
@erobertson42 I won't pretend it's the best example, but this uses Jekyll: https://github.com/tiesont/bootboxdocs |
Great, thank you. I'll take a look when I get a chance. |
Any news about BS5 support in an official release? |
Depends on what you mean by "BS5 support". Properly supporting Bootstrap 5 requires rewriting Bootbox to eliminate our jQuery dependencies, adjust for changes in the JavaScript functions, and figure out how to keep a fair amount of backward compatibility. None of those are trivial tasks, and I haven't had a ton of free time lately. The above changes also necessitate a major version bump, which I'm not entirely sure I want to commit to at this point. @erobertson42 's project looks pretty promising, so if you need something right now, I'd suggest taking a look at https://github.com/erobertson42/bootstrap-modbox Probably not the answer you were looking for, but it's all I have for you (and others) at the moment. |
And really if we're going to add that BS5 support - imho - we should just create new major version, just because we could just drop all the old things with that - there haven't been so much changes in years in this repository so I think that would be the "proper" way to add BS5 support. Although we haven't hear anything from @makeusabrew for a while (we've tried to contact him, but no luck yet - we hope that he's ok), but because we haven't hear anything from him a long time - I think that someone should make fork of this repository and continue working within that with new features. And yes there is couple of those forks already - but I think that we should choose which one to start to use - and recommend that here too. And I really hope that new fork repository maintainers (see the plurar format) could provide their time to maintain and development this great library. |
@tarlepp I did create https://github.com/BootboxJS/bootbox at one point - do you think it makes more sense to just work from that project, since we'd have full control of all settings, or do enough work here to make it fully BS5 compatible and then basically halt development? I do still use Bootbox on a daily basis, but I haven't been able to upgrade those projects to BS5 yet (heck, one is stuck on 3.4 until I can convince the bossmen that it really needs to be updated), so the current Bootbox works just fine for me. I really don't want to abandon this repo, but without @makeusabrew popping in long enough to transfer ownership, it doesn't really make sense to keep working here long term - we can't add any more maintainers or pretty much anything else that requires admin privileges... |
I'm aware of that, but it's also possible to use Bootstrap 5 without jQuery. We use jQuery internally to build the modal (among other things), so for real Bootstrap 5 compatibility, we need to assume jQuery won't be used. |
Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6. - [Release notes](https://github.com/substack/minimist/releases) - [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6) --- updated-dependencies: - dependency-name: minimist dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
….4.1 Bump log4js from 6.3.0 to 6.4.1
…edirects-1.14.8 Bump follow-redirects from 1.14.4 to 1.14.8
…-1.2.6 Bump minimist from 1.2.5 to 1.2.6
Bumps [karma](https://github.com/karma-runner/karma) from 6.3.5 to 6.3.16. - [Release notes](https://github.com/karma-runner/karma/releases) - [Changelog](https://github.com/karma-runner/karma/blob/master/CHANGELOG.md) - [Commits](karma-runner/karma@v6.3.5...v6.3.16) --- updated-dependencies: - dependency-name: karma dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [grunt](https://github.com/gruntjs/grunt) from 1.4.1 to 1.5.2. - [Release notes](https://github.com/gruntjs/grunt/releases) - [Changelog](https://github.com/gruntjs/grunt/blob/main/CHANGELOG) - [Commits](gruntjs/grunt@v1.4.1...v1.5.2) --- updated-dependencies: - dependency-name: grunt dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]>
Bump grunt from 1.4.1 to 1.5.2
Bumps [async](https://github.com/caolan/async) from 3.2.1 to 3.2.3. - [Release notes](https://github.com/caolan/async/releases) - [Changelog](https://github.com/caolan/async/blob/master/CHANGELOG.md) - [Commits](caolan/async@v3.2.1...v3.2.3) --- updated-dependencies: - dependency-name: async dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
Bump async from 3.2.1 to 3.2.3
@jstark518 I don't think I'm going to merge this pull-request. I did fix some of the issues you noted, in the master branch, but I think I want to take a slightly different approach to maintaining Bootstrap 4.6 compatibility. It's up to you at this point whether you want to leave the pull-request open or not. I've deleted and then recreated the v5.x branch, to serve as a snapshot of the current Bootbox version (5.5.2), and am going to spend some time this week working on a basic v6 version. Not going to drop jQuery at this point, but I have a few relatively minor updates I can make to generate the correct markup for both Bootstrap 4 and Bootstrap 5, and add a few features that are in a customized version I use at work. |
Looking forward to it. I like the idea of keeping jQuery. |
Need to delete the repo to make a new fork |