Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PeikyLiu
Copy link

@PeikyLiu PeikyLiu commented Jun 30, 2025

🚀 Feature: Expose updateContainerElements method in useFocusTrap

Description

This PR exposes the updateContainerElements method from the underlying focus-trap library, allowing users to dynamically update focus trap containers while the trap is active.

Related Issues

resolve #4848

📋 Changes

  • Added updateContainerElements method to UseFocusTrapReturn interface
  • Created ContainerElements type alias using Parameters<FocusTrap['updateContainerElements']>[0]
  • Updated return type to handle undefined case when trap is not initialized
  • Added basic container switching demo in demo.vue

🔧 Technical Details

  • Uses TypeScript's Parameters utility type to automatically inherit parameter types from focus-trap library
  • Ensures type consistency with the underlying focus-trap library

📚 Example Usage

const { updateContainerElements, activate } = useFocusTrap(target)
activate()
updateContainerElements(target2.value) // Switch to different container

🔄 Breaking Changes

None - this is a purely additive change.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jun 30, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 4, 2025
Comment on lines +98 to +103
const updateContainerElements = (el: ContainerElements) => {
if (trap) {
trap.updateContainerElements(el)
return trap
}
}
Copy link
Collaborator

@OrbisK OrbisK Jul 6, 2025

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 updateContaienrElements when target changes (instead of activate/deactivate)? wdyt @ilyaliao

Copy link
Member

@ilyaliao ilyaliao Jul 8, 2025

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 updateContaienrElements when target changes (instead of activate/deactivate)? wdyt @ilyaliao

Yes, that's what I initially had in mind as well. We could initialize once with createFocusTrap, and then update the element via updateContainerElements afterwards. But I haven’t looked into the feasibility of this approach in depth yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@PeikyLiu wdyt?

Copy link
Author

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

Copy link
Author

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 updateContaienrElements when 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?

Copy link
Collaborator

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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useFocusTrap | Implement updateContainerElements()
3 participants