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.
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 KFocusTrap and use it in Kmodal #764
add KFocusTrap and use it in Kmodal #764
Changes from 6 commits
18ba260
a37e568
1a30127
dfc92e4
d2fe767
4ce8852
c324310
83e8312
dbec4b6
fb45ecc
bf0c943
02015c7
2aaed51
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you document this prop? You can see other components.
The docstring will then show in the auto-generated
props
documentation on a documentation page for this component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to test that when
component
here isKOverlay
(that is, whenKModal
'sappendToOverlay
prop istrue
), the focus trapping still works as expected.Just from the code I'm a bit suspicious there may be trouble. Reason being that
KOverlay
will move the modal content to another element#k-overlay
, whereasKFocusTrap
will stay in its original place in the DOM.I know I used this structure as an example in the issue, however it was meant to be just rough guidance rather than solution, apologies if that was confusing. Let's preview for both possible
appendToOverlay
values and see if it needs to be adjusted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh , i should have understood the requirements properly, thanks @MisRob i will make sure to use KFocusTrap appropriately.