Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.7k
feat(useFocusTrap): expose updateContainerElements for dynamic contai…#4849
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
| const updateContainerElements = (el: ContainerElements) => { | ||
| if (trap) { | ||
| trap.updateContainerElements(el) | ||
| return trap | ||
| } | ||
| } |
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.
I think we dont want to expose this, but watch the target andupdateContaienrElements when target changes (instead of activate/deactivate)? wdyt@ilyaliao
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.
I think we dont want to expose this, but watch the target and
updateContaienrElementswhen target changes (instead of activate/deactivate)? wdyt@ilyaliao
Yes, that's what I initially had in mind as well. We could initialize once withcreateFocusTrap, and then update the element viaupdateContainerElements afterwards. But I haven’t looked into the feasibility of this approach in depth yet.
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.
@PeikyLiu wdyt?
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.
ok,I'll try based your idea,but it will be take some time
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.
I think we dont want to expose this, but watch the target and
updateContaienrElementswhen target changes (instead of activate/deactivate)? wdyt@ilyaliao
To summarize: you suggest using watch to monitor target changes inside the hook, while keeping the activate/deactivate methods exposed, right?
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 always want to use the target thats passed to the composable. If we want to change the target we want to update the target ref. So we can use updateContainerElements, we should do this while watching target 😄
…kyLiu/vueuse into useFocusTrap/updateContainerElements
Uh oh!
There was an error while loading.Please reload this page.
🚀 Feature: Expose updateContainerElements method in useFocusTrap
Description
This PR exposes the
updateContainerElementsmethod from the underlying focus-trap library, allowing users to dynamically update focus trap containers while the trap is active.Related Issues
resolve#4848
📋 Changes
updateContainerElementsmethod toUseFocusTrapReturninterfaceContainerElementstype alias usingParameters<FocusTrap['updateContainerElements']>[0]undefinedcase when trap is not initialized🔧 Technical Details
Parametersutility type to automatically inherit parameter types from focus-trap library📚 Example Usage
🔄 Breaking Changes
None - this is a purely additive change.