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.

Sign up for GitHub

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

Jump to bottom

feat(useActiveElement): optionally ignore active <body> element #2591

Closed
wants to merge 1 commit into from
Closed

feat(useActiveElement): optionally ignore active <body> element #2591

wants to merge 1 commit into from

Conversation

JMaylor
Copy link
Contributor

@JMaylor JMaylor commented Dec 28, 2022

Description

When focus is switched from some element, elOne to some other element, elTwo to another, the activeElement ref briefly changes to <body>. i.e. the value goes elOne -> <body> -> elTwo. This causes other composables that rely on useActiveElement to also update. e.g. #2214 (useFocusWithin).

While the useActiveElement composable is accurately reflecting what is happening with Document.activeElement, I think that more often than not, this is not the desired outcome. The developer generally thinks:

I am switching from one input in my form to another within my form, therefore I can use useFocusWithin and this should stay true throughout the focus switch.

I added an additional option to useActiveElement, called ignoreBody, which when set to true, will not update activeElement when the focus switches to a (the) body tag. This option is then inherited by useFocusWithin, so that we have:

<script lang="ts" setup>
const { focusedOriginal } = useFocusWithin(target)
// switching focus from elOne to elTwo, focusedOriginal values are the following:
// true -> false -> true

const { focusedNew } = useFocusWithin(target, { ignoreBody: true })
// switching focus from elOne to elTwo, focusedNew values are the following:
// true -> true (unchanged)
</script>

<template>
<form ref="target">
  <input ref="elOne">
  <input ref="elTwo">
</form>
</template>

Additional context

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@vaakian
Copy link
Contributor

vaakian commented Dec 29, 2022

#2581 sloves this without adding extra options.

@dpschen dpschen mentioned this pull request Dec 30, 2022
Merged
9 tasks
@antfu
Copy link
Member

antfu commented Jan 3, 2023

Thank you! Close in favor of #2600

@antfu antfu closed this Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JMaylor @vaakian @antfu