lddubeau / bootprompt Goto Github PK

The documentation needs to be revamped because:

1. It has bugs. I fixed some while producing 5.0 but I'm still finding bugs now, like hyperlinks that link nowhere.

2. It requires a lot of manual work to maintain. A change in the API always requires a change in the documentation. A lot of these double changes should in theory be unnecessary if the API documentation were generated from the TS code with typedoc. There's also the issue that writing something like a list requires <ul class="..."><li class="...">...</li>...</ul> that's a lot of HTML typing. A markdown processor could reduce a lot of this work.

I've deleted composer.json. It was no longer in sync with what bootprompt does. I don't develop with PHP so I don't know how to fix it, etc.

The latest version prior to deletion is here.

If folks want it resurrected someone will have to step up to maintain it.

@tiesont This repo contains the WIP on my fork of Bootbox.

I can make you a collaborator on this repo if you want. That's entirely up to you.

(If you look around the commits you'll see I first named the fork "bootshine", but after a while I found I hated that name, and then renamed again to "bootprompt".)

As I mentioned, I found that over 50 tests did not run in your branch. So when I turned them on, I found some failing tests and fixed the problems. You know much more than I do about the internals so you may have comments about my choices. I'm going to point out the relevant commits below.

1. 9b953d7 This fix brings the code in conformance with what the test suite expected. But more importantly, it provides a nicer message to the user. Without the change, the user would get the error about min having to be lower than max but the real problem is that the other number is NaN.

2. ba1d9c2 The test did not take into account the behavior of a range that has min-max unset. See comment in the commit.

3. 65e24e8 This is similar to the previous commit in that it fixes the test suite so that its expectations conform to the specs that determine how browsers are supposed to behave.

4. fc3a3e7 This test failed before I made this commit. I infer from the test that the intent is that if the user does not specify a default value, then first inputOption should determine the default. However, browsers are such that if the user does not act on the select element, then its value is never changed and remains what it was at creation, an empty string.

The cases above were disconnects between the code and the test suite, but I found an additional issue. I don't think there's a specific line in Bootstrap's documentation that says so, but the de facto standard that I've seen in constructing modals is to take the button which has the class btn-primary as the "default action" for the modal. Being the default action, it should be the button that is focused when the modal shows up. In Bootbox 4, it worked that way. So I set up my modals with btn-primary for the button which is safest. For instance, if someone is about to overwrite data, my app would ask "Do you want to overwrite this data?" and "No" would be the btn-primary. If the user just hits ENTER after the modal shows up, the data has not been overwritten and the user can try again if they really meant to overwrite the data.

On your branch, this is flipped around because the focus is given to the bootbox-accept button. After I started using your branch, all the modals I had that would ask for confirmation before doing something risky would focus the button which makes irreversible changes. In the example I gave above, "Yes" would be focused and if the user just hit ENTER, they'd do the irreversible change. (Yes, there are backups but it's a pain to have to go to a backup to recover the data.)

So in my fork I've changed the code back to focus btn-primary instead of bootprompt-accept.

The onEscape option is a mess. Some problems:

1. The documentation implies that it should be usable on alert, confirm and prompt, but right now it is silently ignored when used with these calls.

2. If the intent was that it should not be used on alert, confirm and prompt, then the code does not show this intent, because there is no test to check whether it has been set before it is overwritten.

3. It is kinda-sorta linked with the processing of the close button (i.e. the button optionally shown in the modal header). If it is set to a callback then the callback will be called both when Escape is hit and when the close button is clicked. However, setting onEscape: false prevents Escape from closing the modal but has no effect on what happens when the close button is clicked. This violates the principle of least surprise.

4. Another violation of the principle of least surprise. Adding to the confusion of the previous point, if onEscape is set to a callback that returns false, then it affects the handling of Escape and of the close button in exactly the same way: neither can close the modal. false should really be a shorthand for () => false and should behave the same way.

There's already evidence above that onEscape should just be allowed on alert, confirm and prompt, but here's an additional argument. An application designer may have decided for whatever reason that the application should not allow users to close modals by hitting escape. There's no reason that alert, confirm and prompt should somehow be excluded from that decision.

Planned changes:

1. Allow the usage of onEscape everywhere.

2. Add a new option onClose which is used only for the close button.

This is a breaking change. Developers will have to check their usage of onEscape and add onClose where needed.

Hello there;

I'm in the process of shifting my focus from trying to maintain Bootbox to creating a new project in an organization (so that there's no "what happens when the owner disappears" problem again). Do you have any interest in joining https://github.com/BootboxJS and merging your work there? You've done most of what I've been wanting to do for a while, in your Typescript version, so it'd be great not to have to revisit work you've already completed.

If not, no offense taken.

The close button on the prompt doesn't have styling.
Bootstrap version 5.2.3

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Repository problems

These problems occurred while renovating this repository. View logs.

• WARN: Using npm packages for Renovate presets is now deprecated. Please migrate to repository-based presets instead.

Datasource	Name	Replacement PR?
npm	popper.js
npm	rollup-plugin-multi-entry
npm	rollup-plugin-terser
npm	tslint

Rate-Limited

These updates are currently rate-limited. Click on a checkbox below to force their creation now.

• build(deps): update dependency font-awesome to v5.15.4
• build(deps): update dependency karma-coverage-istanbul-instrumenter to ^1.0.4
• build(deps): update dependency typescript to ^3.9.10
• build(deps): update dependency chai to ^4.4.1 (chai, @types/chai)
• build(deps): update dependency conventional-changelog-cli to ^2.2.2
• build(deps): update dependency eslint-plugin-import to ^2.29.1
• build(deps): update dependency eslint-plugin-jsx-a11y to ^6.9.0
• build(deps): update dependency karma-chrome-launcher to ^3.2.0
• build(deps): update dependency sinon-chai to ^3.7.0 (sinon-chai, @types/sinon-chai)
• build(deps): update dependency twitter-bootstrap to v4.6.2
• build(deps): update commitlint monorepo to v19 (major) (@commitlint/cli, @commitlint/config-angular)
• build(deps): update dependency @types/sinon to v17
• build(deps): update dependency chai to v5
• build(deps): update dependency conventional-changelog-cli to v5
• build(deps): update dependency eslint to v9
• build(deps): update dependency eslint-plugin-simple-import-sort to v12
• build(deps): update dependency font-awesome to v6
• build(deps): update dependency husky to v9
• build(deps): update dependency mocha to v10 (mocha, @types/mocha)
• build(deps): update dependency rollup to v4
• build(deps): update dependency twitter-bootstrap to v5
• build(deps): update dependency typescript to v5
• 🔐 Create all rate-limited PRs at once 🔐

Edited/Blocked

These updates have been manually edited so Renovate will no longer make changes. To discard all commits and start over, click on a checkbox.

• build(deps): update dependency karma to v6 [security]
• build(deps): update dependency @types/sinon to ^9.0.11
• build(deps): update dependency eslint-plugin-prefer-arrow to ^1.2.3
• build(deps): update dependency husky to ^4.3.8
• build(deps): update dependency @types/bootstrap to ^4.6.6
• build(deps): update dependency eslint-config-lddubeau-base to v6
• build(deps): update dependency karma-firefox-launcher to v2
• build(deps): update dependency rollup-plugin-terser to v7

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• build(deps): update dependency eslint-plugin-react to ^7.34.3
• build(deps): update dependency jquery to v3.7.1 (jquery, @types/jquery)
• build(deps): update dependency rollup to ^2.79.1
• build(deps): update dependency semver to ^7.6.2
• build(deps): update dependency typedoc to ^0.25.13
• build(deps): update dependency @types/bootstrap to v5
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository