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

Added FocusOptions to setFocus function to allow users to pass preventScroll to setFocus #29

Merged
merged 3 commits into from
Dec 12, 2021

Conversation

abhimanyu-singh-uber
Copy link
Contributor

@abhimanyu-singh-uber abhimanyu-singh-uber commented Nov 24, 2021

fixes- theKashey/react-focus-lock#162

@theKashey Please have a look at this. Thanks

@codecov
Copy link

codecov bot commented Nov 24, 2021

Codecov Report

Merging #29 (ef064a2) into master (79a32c1) will decrease coverage by 0.25%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
- Coverage   82.71%   82.46%   -0.26%     
==========================================
  Files          20       20              
  Lines         324      325       +1     
  Branches       70       64       -6     
==========================================
  Hits          268      268              
- Misses         51       52       +1     
  Partials        5        5              
Impacted Files Coverage Δ
src/setFocus.ts 22.72% <40.00%> (-1.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79a32c1...ef064a2. Read the comment docs.

const focusable = getFocusMerge(topNode, lastNode);
const { focusOptions = undefined } = options || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

const { focusOptions } = options || {};

This gives the following TS error
Initializer provides no value for this binding element and the binding element has no default value.

Hence, did it this way.

@theKashey theKashey self-requested a review November 27, 2021 03:40
@theKashey theKashey merged commit 08a9904 into theKashey:master Dec 12, 2021
@theKashey
Copy link
Owner

Looking good

@abhimanyu-singh-uber
Copy link
Contributor Author

yay! Thanks @theKashey let me know when you released the package, I can then raise a PR to react-focus-lock too.

@theKashey
Copy link
Owner

0.10.1 released, jumping to theKashey/react-focus-lock#162
No action is required from your side. Let me handle the merge train 🚂

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