Skip to content
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 ability to pass containerElements to focus-trap #179

Merged
merged 4 commits into from
Nov 17, 2020

Conversation

cgood92
Copy link
Contributor

@cgood92 cgood92 commented Nov 13, 2020

This is WIP until the first PR is merged: focus-trap/focus-trap#217, and the dependency of focus-trap is updated.

Features and Bug Fixes

  • Unit test coverage added/updated.
  • E2E test coverage added/updated.
  • Prop-types added/updated.
  • Typings added/updated.
  • README updated (API changes, instructions, etc.).
  • Changes to dependencies explained.
  • Changeset added (run yarn changeset locally to add one, follow prompts).

This is part 2 of the PR focus-trap/focus-trap#217


Demo 1
demo2

Demo 2
demo

@changeset-bot
Copy link

changeset-bot bot commented Nov 13, 2020

🦋 Changeset detected

Latest commit: afccf7d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
focus-trap-react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cgood92 cgood92 changed the title Add ability to pass containerElements to focus-trap WIP: Add ability to pass containerElements to focus-trap Nov 13, 2020
Copy link
Member

@stefcameron stefcameron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgood92 I appreciate the update in this repo too, ahead of the pending related focus-trap changes. That's a big help! This seems solid, just a couple of minor things. I also really appreciate you took the time to add unit and 2e2 tests! 💪

.changeset/young-crews-report.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/focus-trap-react.js Outdated Show resolved Hide resolved
@cgood92 cgood92 changed the title WIP: Add ability to pass containerElements to focus-trap Add ability to pass containerElements to focus-trap Nov 16, 2020
@cgood92
Copy link
Contributor Author

cgood92 commented Nov 16, 2020

@stefcameron thank you so much for your time and consideration. This PR is now ready to go, with the new release of focus-trap.

@stefcameron
Copy link
Member

@stefcameron thank you so much for your time and consideration. This PR is now ready to go, with the new release of focus-trap.

My pleasure! Thanks for updating it to the latest focus-trap release. I'll check this out ASAP!

.changeset/young-crews-report.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@stefcameron stefcameron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could of things...

index.d.ts Outdated Show resolved Hide resolved
Copy link
Member

@stefcameron stefcameron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this, @cgood92, much appreciated! 🎉

@stefcameron stefcameron merged commit 76ed007 into focus-trap:master Nov 17, 2020
@stefcameron
Copy link
Member

@all-contributors please add @cgood92 for code, docs, examples, tests

@allcontributors
Copy link
Contributor

@stefcameron

I've put up a pull request to add @cgood92! 🎉

@github-actions github-actions bot mentioned this pull request Nov 17, 2020
@stefcameron
Copy link
Member

Published in 8.2.0

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.

2 participants