ts: fix TypeScript syntax for allowOutsideClick option#123
Merged
stefcameron merged 1 commit intofocus-trap:masterfrom Aug 12, 2020
Merged
ts: fix TypeScript syntax for allowOutsideClick option#123stefcameron merged 1 commit intofocus-trap:masterfrom
stefcameron merged 1 commit intofocus-trap:masterfrom
Conversation
💥 No ChangesetLatest commit: dddc8de Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂 If these changes should be published to npm, you need to add a changeset. Click here to learn what changesets are, and how to add one. Click here if you're a maintainer who wants to add a changeset to this PR |
Member
|
@michael-ar Thanks for spotting that! TS is not my forte, though I would never have thought |
Contributor
Author
|
@stefcameron all good — I would have intuitively thought so too! do you have any idea when the next npm release might happen? am currently installing directly from GitHub as I need one of the unreleased options |
Member
😅 What's the best way to verify the typings? Run the file through the TS compiler somehow, or through typescript-eslint? I'd like to add some simple tooling to validate the syntax as part of the quality process.
I'm currently working on #124 and then will publish an update, most likely 6.0.0 just to be safe because of the path change to |
stefcameron
added a commit
that referenced
this pull request
Aug 15, 2020
Fixes #124 to put this repo inline with focus-trap-react: - update all deps to latest versions - move to yarn with yarn.lock - use Webpack to build instead of Browserify (see below for more details) - update Prettier and run on all the source (same rules as focus-trap-react) - add check for types so that problems like #123 don't happen again - (*) fixes bug in package.json 'main' that wasn't pointing to /dist/focus-trap.js (build output) Because of (*) this should be published as a new __major__ release, just to be safe, even though there shouldn't be any breaking changes as a result (famous last words...). This should also close-out a bunch of the security-related notices on the repo from Dependabot because of old package versions with vulnerabilities. Added Webpack builds and removed some deps: - Webpack Dev and Prod builds, mimicing old Browserify builds. - Removed Browserify: Webpack replaces. - Removed uglify: Webpack takes care of this now. - Removed xtend: simple ES6 object merging works fine. Fixed 2 small bugs: - Fixed bug on line 316 of index.js where an NPE would happen if the trap was created without `userOptions`; should be `config.preventScroll` instead since we create `config` early on within the module and use that everywhere instead of `userOptions`. - Fixed bug in allow-click-outside demo by disabling ESC: Otherwise, the 'activate trap' button state would get out of sync with the state of the trap (would remain 'deactivate trap' after pressing ESC). NOTE: Publishing is still done manually from a local repo. If we move this repo to Changesets later, we can start doing releases via GitHub Actions like `focus-trap-react`.
Member
I figured out it's just to add |
stefcameron
added a commit
that referenced
this pull request
Aug 15, 2020
Fixes #124 to put this repo inline with focus-trap-react: - update all deps to latest versions - move to yarn with yarn.lock - use Webpack to build instead of Browserify (see below for more details) - update Prettier and run on all the source (same rules as focus-trap-react) - add check for types so that problems like #123 don't happen again - (*) fixes bug in package.json 'main' that wasn't pointing to /dist/focus-trap.js (build output) Because of (*) this should be published as a new __major__ release, just to be safe, even though there shouldn't be any breaking changes as a result (famous last words...). This should also close-out a bunch of the security-related notices on the repo from Dependabot because of old package versions with vulnerabilities. Added Webpack builds and removed some deps: - Webpack Dev and Prod builds, mimicing old Browserify builds. - Removed Browserify: Webpack replaces. - Removed uglify: Webpack takes care of this now. - Removed xtend: simple ES6 object merging works fine. Fixed 2 small bugs: - Fixed bug on line 316 of index.js where an NPE would happen if the trap was created without `userOptions`; should be `config.preventScroll` instead since we create `config` early on within the module and use that everywhere instead of `userOptions`. - Fixed bug in allow-click-outside demo by disabling ESC: Otherwise, the 'activate trap' button state would get out of sync with the state of the trap (would remain 'deactivate trap' after pressing ESC). NOTE: Publishing is still done manually from a local repo. If we move this repo to Changesets later, we can start doing releases via GitHub Actions like `focus-trap-react`.
stefcameron
added a commit
that referenced
this pull request
Aug 15, 2020
Fixes #124 to put this repo inline with focus-trap-react: - update all deps to latest versions - move to yarn with yarn.lock - use Webpack to build instead of Browserify (see below for more details) - update Prettier and run on all the source (same rules as focus-trap-react) - add check for types so that problems like #123 don't happen again - (*) fixes bug in package.json 'main' that wasn't pointing to /dist/focus-trap.js (build output) Because of (*) this should be published as a new __major__ release, just to be safe, even though there shouldn't be any breaking changes as a result (famous last words...). This should also close-out a bunch of the security-related notices on the repo from Dependabot because of old package versions with vulnerabilities. Added Webpack builds and removed some deps: - Webpack Dev and Prod builds, mimicing old Browserify builds. - Removed Browserify: Webpack replaces. - Removed uglify: Webpack takes care of this now. - Removed xtend: simple ES6 object merging works fine. Fixed 2 small bugs: - Fixed bug on line 316 of index.js where an NPE would happen if the trap was created without `userOptions`; should be `config.preventScroll` instead since we create `config` early on within the module and use that everywhere instead of `userOptions`. - Fixed bug in allow-click-outside demo by disabling ESC: Otherwise, the 'activate trap' button state would get out of sync with the state of the trap (would remain 'deactivate trap' after pressing ESC). NOTE: Publishing is still done manually from a local repo. If we move this repo to Changesets later, we can start doing releases via GitHub Actions like `focus-trap-react`.
stefcameron
added a commit
that referenced
this pull request
Aug 17, 2020
Fixes #124 to put this repo inline with focus-trap-react: - update all deps to latest versions - move to yarn with yarn.lock - use Webpack to build instead of Browserify (see below for more details) - update Prettier and run on all the source (same rules as focus-trap-react) - add check for types so that problems like #123 don't happen again - (*) fixes bug in package.json 'main' that wasn't pointing to /dist/focus-trap.js (build output) Because of (*) this should be published as a new __major__ release, just to be safe, even though there shouldn't be any breaking changes as a result (famous last words...). This should also close-out a bunch of the security-related notices on the repo from Dependabot because of old package versions with vulnerabilities. Added Webpack builds and removed some deps: - Webpack Dev and Prod builds, mimicing old Browserify builds. - Removed Browserify: Webpack replaces. - Removed uglify: Webpack takes care of this now. - Removed xtend: simple ES6 object merging works fine. Fixed 2 small bugs: - Fixed bug on line 316 of index.js where an NPE would happen if the trap was created without `userOptions`; should be `config.preventScroll` instead since we create `config` early on within the module and use that everywhere instead of `userOptions`. - Fixed bug in allow-click-outside demo by disabling ESC: Otherwise, the 'activate trap' button state would get out of sync with the state of the trap (would remain 'deactivate trap' after pressing ESC). NOTE: Publishing is still done manually from a local repo. If we move this repo to Changesets later, we can start doing releases via GitHub Actions like `focus-trap-react`.
Contributor
Author
Oh nice one, definitely nice to have as part of the automated flow. Thanks again! |
Contributor
@stefcameron That makes three of us! I'll file that away for next time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recent PR contains invalid syntax for new option, this fixes.