Skip to content

Conversation

@Westbrook
Copy link
Contributor

Description

When a hover overlay is open, if there is no click overlay content associated to its trigger it should persist when clicking on the trigger.

Move logic for making this true out of the overlay-trigger element, and into the overlay-stack so that it doesn't cause unexpected side effects and can be leveraged by the entire overlay API.

Added bonus, deduplicate the sync and async test code.

Related issue(s)

How has this been tested?

  • Test case 1
    1. Go here
    2. Follow the instructions in the story UI
    3. Ensure the hover content is still available at the end of the workflow.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Comment on lines +595 to +596
!event.composedPath().includes(overlay.trigger) ||
overlay.interaction !== 'hover'
Copy link
Member

Choose a reason for hiding this comment

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

Perfect. This should keep overlays open that are in the same path (i.e. an open modal that has a click overlay), and it should keep hover overlays open as well (they go away when you remove hover). NAILED IT!

Copy link
Member

@najikahalsema najikahalsema left a comment

Choose a reason for hiding this comment

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

Larry beat me to it! Great work!

@github-actions
Copy link
Contributor

Tachometer results

Chrome

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 31.59ms - 31.88ms - faster ✔
1% - 2%
0.20ms - 0.54ms
branch 244 kB 32.02ms - 32.19ms slower ❌
1% - 2%
0.20ms - 0.54ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 183.93ms - 185.03ms - faster ✔
0% - 1%
0.22ms - 2.56ms
branch 607 kB 184.84ms - 186.91ms slower ❌
0% - 1%
0.22ms - 2.56ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 297 kB 209.66ms - 211.92ms - unsure 🔍
-1% - +0%
-1.64ms - +0.84ms
branch 309 kB 210.67ms - 211.71ms unsure 🔍
-0% - +1%
-0.84ms - +1.64ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 70.94ms - 71.21ms - faster ✔
1% - 3%
0.39ms - 2.11ms
branch 347 kB 71.48ms - 73.18ms slower ❌
1% - 3%
0.39ms - 2.11ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 674.33ms - 678.90ms - faster ✔
0% - 1%
0.37ms - 6.86ms
branch 454 kB 677.93ms - 682.54ms slower ❌
0% - 1%
0.37ms - 6.86ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 31.36ms - 31.53ms - faster ✔
1% - 2%
0.35ms - 0.63ms
branch 241 kB 31.82ms - 32.05ms slower ❌
1% - 2%
0.35ms - 0.63ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 1946.39ms - 1949.19ms - unsure 🔍
-0% - +0%
-3.70ms - +0.35ms
branch 546 kB 1947.99ms - 1950.94ms unsure 🔍
-0% - +0%
-0.35ms - +3.70ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 33.48ms - 33.61ms - faster ✔
1% - 2%
0.36ms - 0.54ms
branch 247 kB 33.93ms - 34.05ms slower ❌
1% - 2%
0.36ms - 0.54ms
-
Firefox

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 137.53ms - 151.43ms - unsure 🔍
-6% - +7%
-9.00ms - +9.76ms
branch 244 kB 137.81ms - 150.39ms unsure 🔍
-7% - +6%
-9.76ms - +9.00ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 611.86ms - 626.26ms - unsure 🔍
-2% - +1%
-10.96ms - +8.56ms
branch 607 kB 613.67ms - 626.85ms unsure 🔍
-1% - +2%
-8.56ms - +10.96ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 297 kB 794.21ms - 809.59ms - unsure 🔍
-2% - +0%
-16.69ms - +3.85ms
branch 309 kB 801.51ms - 815.13ms unsure 🔍
-0% - +2%
-3.85ms - +16.69ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 261.03ms - 272.77ms - unsure 🔍
-5% - +1%
-13.91ms - +2.35ms
branch 347 kB 267.05ms - 278.31ms unsure 🔍
-1% - +5%
-2.35ms - +13.91ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 1822.92ms - 1846.96ms - unsure 🔍
-1% - +1%
-25.25ms - +10.73ms
branch 454 kB 1828.81ms - 1855.59ms unsure 🔍
-1% - +1%
-10.73ms - +25.25ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 112.30ms - 134.42ms - unsure 🔍
-7% - +19%
-7.45ms - +21.49ms
branch 241 kB 107.02ms - 125.66ms unsure 🔍
-17% - +6%
-21.49ms - +7.45ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 2178.00ms - 2208.16ms - unsure 🔍
-1% - +1%
-12.51ms - +26.75ms
branch 546 kB 2173.39ms - 2198.53ms unsure 🔍
-1% - +1%
-26.75ms - +12.51ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 115.14ms - 130.58ms - unsure 🔍
-6% - +10%
-7.07ms - +12.15ms
branch 247 kB 114.61ms - 126.03ms unsure 🔍
-10% - +6%
-12.15ms - +7.07ms
-

@Westbrook Westbrook merged commit 67c7066 into main Sep 23, 2022
@Westbrook Westbrook deleted the pure-overlay-trigger branch September 23, 2022 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: clicking on an overlay-trigger with hover content only does not close overlays triggered by click

4 participants