feat: Add boolean value support for allowOutsideClick option#120
feat: Add boolean value support for allowOutsideClick option#120stefcameron merged 4 commits intofocus-trap:masterfrom
Conversation
This work follows a discussion from focus-trap/focus-trap-react#86.
💥 No ChangesetLatest commit: efdc540 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 |
stefcameron
left a comment
There was a problem hiding this comment.
@SeanMcP Thanks for quickly opening this PR! Everything looks OK on the surface. Thanks for adding that test in the demo. I'll run that manually myself later just to double-check before I give approval. For now, and while we get Changesets sorted out in this repo, could you please add a line to the "UNRELEASED" section of the CHANGELOG.md file as part of this PR?
|
@stefcameron Done in f0f6133 |
stefcameron
left a comment
There was a problem hiding this comment.
This all looks great to me! I'll pull it in the next couple of days. Ran out of time today.
| return true; | ||
| function initialize() { | ||
| return createFocusTrap('#allowoutsideclick', { | ||
| allowOutsideClick: allowOutsideClick, |
There was a problem hiding this comment.
This can be shortened to just allowOutsideClick. :)
There was a problem hiding this comment.
Good catch! I had that initially, but there was a parsing error when I ran npm run lint:
/Users/seanmcp/dev/seanmcp/focus-trap/demo/js/allowoutsideclick.js
10:22 error Parsing error: Unexpected token ,
✖ 1 problem (1 error, 0 warnings)
I couldn't figure out what was wrong, and it didn't seem worth the stress for a demo.
There was a problem hiding this comment.
No worries. I haven't updated all the deps and tooling on this repo like I have on the react one so there's probably some babel-eslint thing missing.
Description
This adds support for boolean value to be passed to the
allowOutsideClickoption. This addresses focus-trap/focus-trap-react#86 and is the follow-up to focus-trap/focus-trap-react#87.Considerations
Documentation
When describing the type of
allowOutsideClick, I went with "If set and is or returnstrue". There may be a better wording for this.Demo
I wanted to add an example without cluttering the page with another example that is visually identical. I opted for a
selectelement to switch between passing a function or a boolean. This is contained in ceec9da.