From 0c0601f87a13d8457d7d0427e521d751c5bd7b0b Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 10 Mar 2023 22:00:35 +0100 Subject: [PATCH] Fix focus styles showing up when using the mouse (#2347) * update playground examples to use a shared `Button` * expose a `ui-focus-visible` variant * keep track of a `data-headlessui-focus-visible` attribute * do not set the `tabindex` The focus was always set, but the ring wasn't showing up. This was also focusing a ring when the browser decided not the add one. Let's make the browser decide when to show this or not. * update changelog --- packages/@headlessui-react/CHANGELOG.md | 4 +- .../src/utils/focus-management.ts | 50 +++++++++++--- packages/@headlessui-tailwindcss/package.json | 2 +- .../@headlessui-tailwindcss/src/index.test.ts | 16 +++++ packages/@headlessui-tailwindcss/src/index.ts | 2 + packages/@headlessui-vue/CHANGELOG.md | 4 +- .../src/utils/focus-management.ts | 50 +++++++++++--- .../playground-react/components/button.tsx | 20 ++++++ packages/playground-react/package.json | 2 +- .../pages/combinations/form.tsx | 39 +++++------ .../pages/combinations/tabs-in-dialog.tsx | 15 ++-- .../combobox/combobox-with-pure-tailwind.tsx | 3 +- .../pages/dialog/dialog-focus-issue.tsx | 14 ++-- .../dialog/dialog-with-shadow-children.tsx | 8 +-- .../playground-react/pages/dialog/dialog.tsx | 69 +++++-------------- .../pages/menu/menu-with-framer-motion.tsx | 26 ++++--- .../pages/menu/menu-with-popper.tsx | 6 +- .../menu/menu-with-transition-and-popper.tsx | 6 +- .../pages/menu/menu-with-transition.tsx | 7 +- packages/playground-react/pages/menu/menu.tsx | 3 +- .../pages/menu/multiple-elements.tsx | 3 +- .../transitions/component-examples/modal.tsx | 4 -- .../component-examples/peek-a-boo.tsx | 6 -- .../playground-vue/src/components/Button.vue | 9 +++ .../src/components/dialog/dialog.vue | 68 +++++------------- yarn.lock | 38 ++++++---- 26 files changed, 258 insertions(+), 216 deletions(-) create mode 100644 packages/playground-react/components/button.tsx create mode 100644 packages/playground-vue/src/components/Button.vue diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index fc98b4ee3e..462d61357b 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Fix focus styles showing up when using the mouse ([#2347](https://github.com/tailwindlabs/headlessui/pull/2347)) ## [1.7.13] - 2023-03-03 diff --git a/packages/@headlessui-react/src/utils/focus-management.ts b/packages/@headlessui-react/src/utils/focus-management.ts index 33016645b4..fb4fe517ad 100644 --- a/packages/@headlessui-react/src/utils/focus-management.ts +++ b/packages/@headlessui-react/src/utils/focus-management.ts @@ -116,6 +116,47 @@ export function restoreFocusIfNecessary(element: HTMLElement | null) { }) } +// The method of triggering an action, this is used to determine how we should +// restore focus after an action has been performed. +enum ActivationMethod { + /* If the action was triggered by a keyboard event. */ + Keyboard = 0, + + /* If the action was triggered by a mouse / pointer / ... event.*/ + Mouse = 1, +} + +// We want to be able to set and remove the `data-headlessui-mouse` attribute on the `html` element. +if (typeof window !== 'undefined' && typeof document !== 'undefined') { + document.addEventListener( + 'keydown', + (event) => { + if (event.metaKey || event.altKey || event.ctrlKey) { + return + } + + document.documentElement.dataset.headlessuiFocusVisible = '' + }, + true + ) + + document.addEventListener( + 'click', + (event) => { + // Event originated from an actual mouse click + if (event.detail === ActivationMethod.Mouse) { + delete document.documentElement.dataset.headlessuiFocusVisible + } + + // Event originated from a keyboard event that triggered the `click` event + else if (event.detail === ActivationMethod.Keyboard) { + document.documentElement.dataset.headlessuiFocusVisible = '' + } + }, + true + ) +} + export function focusElement(element: HTMLElement | null) { element?.focus({ preventScroll: true }) } @@ -232,14 +273,5 @@ export function focusIn( next.select() } - // This is a little weird, but let me try and explain: There are a few scenario's - // in chrome for example where a focused `` tag does not get the default focus - // styles and sometimes they do. This highly depends on whether you started by - // clicking or by using your keyboard. When you programmatically add focus `anchor.focus()` - // then the active element (document.activeElement) is this anchor, which is expected. - // However in that case the default focus styles are not applied *unless* you - // also add this tabindex. - if (!next.hasAttribute('tabindex')) next.setAttribute('tabindex', '0') - return FocusResult.Success } diff --git a/packages/@headlessui-tailwindcss/package.json b/packages/@headlessui-tailwindcss/package.json index 77fe68af41..898386eb59 100644 --- a/packages/@headlessui-tailwindcss/package.json +++ b/packages/@headlessui-tailwindcss/package.json @@ -37,6 +37,6 @@ }, "devDependencies": { "esbuild": "^0.11.18", - "tailwindcss": "^3.2.4" + "tailwindcss": "^3.2.7" } } diff --git a/packages/@headlessui-tailwindcss/src/index.test.ts b/packages/@headlessui-tailwindcss/src/index.test.ts index e896ae34bd..375d31c6ba 100644 --- a/packages/@headlessui-tailwindcss/src/index.test.ts +++ b/packages/@headlessui-tailwindcss/src/index.test.ts @@ -9,6 +9,7 @@ let css = String.raw function run(input: string, config: any, plugin = tailwind) { let { currentTestName } = expect.getState() + // @ts-ignore return postcss(plugin(config)).process(input, { from: `${path.resolve(__filename)}?test=${currentTestName}`, }) @@ -52,6 +53,21 @@ it('should generate the inverse "not" css for an exposed state', async () => { }) }) +it('should generate the ui-focus-visible variant', async () => { + let config = { + content: [{ raw: html`
` }], + plugins: [hui], + } + + return run('@tailwind utilities', config).then((result) => { + expect(result.css).toMatchFormattedCss(css` + :where([data-headlessui-focus-visible]) .ui-focus-visible\:underline:focus { + text-decoration-line: underline; + } + `) + }) +}) + describe('custom prefix', () => { it('should generate css for an exposed state', async () => { let config = { diff --git a/packages/@headlessui-tailwindcss/src/index.ts b/packages/@headlessui-tailwindcss/src/index.ts index b825eea7fe..d30d52e720 100644 --- a/packages/@headlessui-tailwindcss/src/index.ts +++ b/packages/@headlessui-tailwindcss/src/index.ts @@ -31,6 +31,8 @@ export default plugin.withOptions(({ prefix = 'ui' } = {}) => { `&[data-headlessui-state]:not([data-headlessui-state~="${state}"])`, `:where([data-headlessui-state]:not([data-headlessui-state~="${state}"])) &:not([data-headlessui-state])`, ]) + + addVariant(`${prefix}-focus-visible`, ':where([data-headlessui-focus-visible]) &:focus') } } }) diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 807c13518d..8f938fd986 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +### Fixed + +- Fix focus styles showing up when using the mouse ([#2347](https://github.com/tailwindlabs/headlessui/pull/2347)) ## [1.7.12] - 2023-03-03 diff --git a/packages/@headlessui-vue/src/utils/focus-management.ts b/packages/@headlessui-vue/src/utils/focus-management.ts index 846f666562..0b68f7dda3 100644 --- a/packages/@headlessui-vue/src/utils/focus-management.ts +++ b/packages/@headlessui-vue/src/utils/focus-management.ts @@ -109,6 +109,47 @@ export function restoreFocusIfNecessary(element: HTMLElement | null) { }) } +// The method of triggering an action, this is used to determine how we should +// restore focus after an action has been performed. +enum ActivationMethod { + /* If the action was triggered by a keyboard event. */ + Keyboard = 0, + + /* If the action was triggered by a mouse / pointer / ... event.*/ + Mouse = 1, +} + +// We want to be able to set and remove the `data-headlessui-mouse` attribute on the `html` element. +if (typeof window !== 'undefined' && typeof document !== 'undefined') { + document.addEventListener( + 'keydown', + (event) => { + if (event.metaKey || event.altKey || event.ctrlKey) { + return + } + + document.documentElement.dataset.headlessuiFocusVisible = '' + }, + true + ) + + document.addEventListener( + 'click', + (event) => { + // Event originated from an actual mouse click + if (event.detail === ActivationMethod.Mouse) { + delete document.documentElement.dataset.headlessuiFocusVisible + } + + // Event originated from a keyboard event that triggered the `click` event + else if (event.detail === ActivationMethod.Keyboard) { + document.documentElement.dataset.headlessuiFocusVisible = '' + } + }, + true + ) +} + export function focusElement(element: HTMLElement | null) { element?.focus({ preventScroll: true }) } @@ -226,14 +267,5 @@ export function focusIn( next.select() } - // This is a little weird, but let me try and explain: There are a few scenario's - // in chrome for example where a focused `` tag does not get the default focus - // styles and sometimes they do. This highly depends on whether you started by - // clicking or by using your keyboard. When you programmatically add focus `anchor.focus()` - // then the active element (document.activeElement) is this anchor, which is expected. - // However in that case the default focus styles are not applied *unless* you - // also add this tabindex. - if (!next.hasAttribute('tabindex')) next.setAttribute('tabindex', '0') - return FocusResult.Success } diff --git a/packages/playground-react/components/button.tsx b/packages/playground-react/components/button.tsx new file mode 100644 index 0000000000..fa9cdb5d3e --- /dev/null +++ b/packages/playground-react/components/button.tsx @@ -0,0 +1,20 @@ +import { ComponentProps, forwardRef, ReactNode } from 'react' + +function classNames(...classes: (string | false | undefined | null)[]) { + return classes.filter(Boolean).join(' ') +} + +export let Button = forwardRef< + HTMLButtonElement, + ComponentProps<'button'> & { children?: ReactNode } +>(({ className, ...props }, ref) => ( + +
+
- - Tab 1 - Tab 2 - Tab 3 + + Tab 1 + Tab 2 + Tab 3 Panel 1 @@ -26,6 +27,6 @@ export default function App() {
- +
) } diff --git a/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx b/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx index 8e76c46b3d..e97cfafd3d 100644 --- a/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx +++ b/packages/playground-react/pages/combobox/combobox-with-pure-tailwind.tsx @@ -2,6 +2,7 @@ import React, { useState, useEffect } from 'react' import { Combobox } from '@headlessui/react' import { classNames } from '../../utils/class-names' +import { Button } from '../../components/button' let everybody = [ 'Wade Cooper', @@ -64,7 +65,7 @@ export default function Home() { onChange={(e) => setQuery(e.target.value)} className="border-none px-3 py-1 outline-none" /> - +
- - - + + +
@@ -23,12 +24,7 @@ export default function DialogFocusIssue() { return (

Headless UI Focus Jump

- +
diff --git a/packages/playground-react/pages/dialog/dialog-with-shadow-children.tsx b/packages/playground-react/pages/dialog/dialog-with-shadow-children.tsx index 61261c564b..f37a248026 100644 --- a/packages/playground-react/pages/dialog/dialog-with-shadow-children.tsx +++ b/packages/playground-react/pages/dialog/dialog-with-shadow-children.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef } from 'react' import { useState } from 'react' import { Dialog } from '@headlessui/react' +import { Button } from '../../components/button' if (typeof document !== 'undefined') { class MyCustomElement extends HTMLElement { @@ -49,12 +50,7 @@ export default function App() { return (
- + setOpen(false)}>
diff --git a/packages/playground-react/pages/dialog/dialog.tsx b/packages/playground-react/pages/dialog/dialog.tsx index b617b59bd3..64d10ec9b0 100644 --- a/packages/playground-react/pages/dialog/dialog.tsx +++ b/packages/playground-react/pages/dialog/dialog.tsx @@ -3,6 +3,7 @@ import Flatpickr from 'react-flatpickr' import { Dialog, Menu, Portal, Transition } from '@headlessui/react' import { usePopper } from '../../utils/hooks/use-popper' import { classNames } from '../../utils/class-names' +import { Button } from '../../components/button' import 'flatpickr/dist/themes/light.css' @@ -14,16 +15,6 @@ function resolveClass({ active, disabled }) { ) } -function Button(props: React.ComponentProps<'button'>) { - return ( - @@ -70,11 +61,6 @@ export default function Home() {
{nested && setNested(false)} />} -
- -
+
- - + Choose a reason + - Choose a reason - - - - - + + +
-
- - +
+ +
diff --git a/packages/playground-react/pages/menu/menu-with-framer-motion.tsx b/packages/playground-react/pages/menu/menu-with-framer-motion.tsx index e68263c6c3..c51d158390 100644 --- a/packages/playground-react/pages/menu/menu-with-framer-motion.tsx +++ b/packages/playground-react/pages/menu/menu-with-framer-motion.tsx @@ -1,9 +1,10 @@ -import React from 'react' +import React, { forwardRef } from 'react' import Link from 'next/link' -import { Menu } from '@headlessui/react' +import { Menu, MenuItemProps } from '@headlessui/react' import { AnimatePresence, motion } from 'framer-motion' import { classNames } from '../../utils/class-names' +import { Button } from '../../components/button' export default function Home() { return ( @@ -13,7 +14,7 @@ export default function Home() { {({ open }) => ( <> - + Options ) { +let NextLink = forwardRef((props: React.ComponentProps<'a'>, ref) => { let { href, children, ...rest } = props return ( - {children} + + {children} + ) -} +}) -function SignOutButton(props) { +let SignOutButton = forwardRef((props, ref) => { return (
-
) -} +}) -function Item(props) { +let Item = forwardRef>((props, ref) => { return ( classNames( @@ -108,4 +112,4 @@ function Item(props) { {...props} /> ) -} +}) diff --git a/packages/playground-react/pages/menu/menu-with-popper.tsx b/packages/playground-react/pages/menu/menu-with-popper.tsx index e8ee320c91..43bab7a917 100644 --- a/packages/playground-react/pages/menu/menu-with-popper.tsx +++ b/packages/playground-react/pages/menu/menu-with-popper.tsx @@ -4,6 +4,7 @@ import { Menu } from '@headlessui/react' import { usePopper } from '../../utils/hooks/use-popper' import { classNames } from '../../utils/class-names' +import { Button } from '../../components/button' export default function Home() { let [trigger, container] = usePopper({ @@ -25,10 +26,7 @@ export default function Home() {
- + Options - + Options
-
- + Options - + Options - + Options -
diff --git a/packages/playground-react/pages/transitions/component-examples/peek-a-boo.tsx b/packages/playground-react/pages/transitions/component-examples/peek-a-boo.tsx index 1deeee541f..149c419ff8 100644 --- a/packages/playground-react/pages/transitions/component-examples/peek-a-boo.tsx +++ b/packages/playground-react/pages/transitions/component-examples/peek-a-boo.tsx @@ -18,12 +18,6 @@ export default function Home() { -