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.

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

fix: match "pointerup" listeners with "pointercancel" for full coverage #2597

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

Westbrook
Copy link
Contributor

Description

Ensure all pointerup listeners are matched with a pointercancel listener in case the visitor proceeds to scroll while the pointer is down.

How has this been tested?

Types of changes

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

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

Copy link
Contributor

@hunterloftis hunterloftis left a comment

Choose a reason for hiding this comment

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

It's such a pity to have to lift the responsibility for all this pointer-state handling into JS. I know this was re: a browser bug... maybe something we could track to subsequently cut all of this?

One other approach to consider, now that the bookkeeping list is getting pretty large, is using an AbortController signal to remove batches of event listeners instead of having to manually keep them in sync.

@Westbrook
Copy link
Contributor Author

Yeah, for larger number of matched listeners #1877 does make a lot of sense.

Some of these are JS first problems, others (like synthetic :active) come from accessibility issues with <button> elements within shadow roots. https://w3c.github.io/webcomponents-cg/2022.html#cross-root-aria will hopefully give us that capability back, at some point...

@Westbrook Westbrook merged commit 7f2ce92 into main Sep 19, 2022
@Westbrook Westbrook deleted the pointercancel branch September 19, 2022 16:16
@github-actions
Copy link

github-actions bot commented Sep 19, 2022

Tachometer results

Chrome

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 34.08ms - 34.45ms - unsure 🔍
-1% - +1%
-0.35ms - +0.36ms
branch 234 kB 33.95ms - 34.56ms unsure 🔍
-1% - +1%
-0.36ms - +0.35ms
-

action-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 400 kB 58.55ms - 61.30ms - unsure 🔍
-2% - +4%
-1.23ms - +2.14ms
branch 402 kB 58.50ms - 60.44ms unsure 🔍
-4% - +2%
-2.14ms - +1.23ms
-

action-group permalink

Version Bytes Avg Time vs remote vs branch
npm latest 426 kB 115.37ms - 116.75ms - unsure 🔍
-1% - +1%
-0.64ms - +1.29ms
branch 428 kB 115.07ms - 116.40ms unsure 🔍
-1% - +1%
-1.29ms - +0.64ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 219.49ms - 221.40ms - faster ✔
0% - 2%
0.19ms - 3.65ms
branch 597 kB 220.92ms - 223.81ms slower ❌
0% - 2%
0.19ms - 3.65ms
-

button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 321 kB 52.71ms - 53.08ms - faster ✔
0% - 2%
0.11ms - 0.88ms
branch 323 kB 53.05ms - 53.72ms slower ❌
0% - 2%
0.11ms - 0.88ms
-

card permalink

Version Bytes Avg Time vs remote vs branch
npm latest 379 kB 98.61ms - 99.83ms - unsure 🔍
-1% - +1%
-1.12ms - +0.93ms
branch 381 kB 98.49ms - 100.14ms unsure 🔍
-1% - +1%
-0.93ms - +1.12ms
-

dialog permalink

Version Bytes Avg Time vs remote vs branch
npm latest 306 kB 81.95ms - 82.71ms - unsure 🔍
-2% - +0%
-1.57ms - +0.38ms
branch 307 kB 82.03ms - 83.82ms unsure 🔍
-0% - +2%
-0.38ms - +1.57ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 297 kB 248.18ms - 250.89ms - unsure 🔍
-2% - +0%
-5.44ms - +0.32ms
branch 299 kB 249.55ms - 254.64ms unsure 🔍
-0% - +2%
-0.32ms - +5.44ms
-

number-field permalink

Version Bytes Avg Time vs remote vs branch
npm latest 492 kB 188.45ms - 189.89ms - unsure 🔍
-0% - +1%
-0.60ms - +1.33ms
branch 494 kB 188.16ms - 189.45ms unsure 🔍
-1% - +0%
-1.33ms - +0.60ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 80.56ms - 81.25ms - unsure 🔍
-1% - +0%
-0.86ms - +0.28ms
branch 337 kB 80.75ms - 81.65ms unsure 🔍
-0% - +1%
-0.28ms - +0.86ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 784.08ms - 800.25ms - unsure 🔍
-2% - +1%
-16.83ms - +7.51ms
branch 443 kB 787.73ms - 805.92ms unsure 🔍
-1% - +2%
-7.51ms - +16.83ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 34.82ms - 35.07ms - unsure 🔍
-1% - +0%
-0.20ms - +0.15ms
branch 231 kB 34.85ms - 35.09ms unsure 🔍
-0% - +1%
-0.15ms - +0.20ms
-

search permalink

Version Bytes Avg Time vs remote vs branch
npm latest 329 kB 94.43ms - 95.20ms - unsure 🔍
-2% - +0%
-1.46ms - +0.17ms
branch 331 kB 94.74ms - 96.18ms unsure 🔍
-0% - +2%
-0.17ms - +1.46ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 330 kB 150.20ms - 151.23ms - unsure 🔍
-1% - +1%
-1.48ms - +0.90ms
branch 331 kB 149.93ms - 152.07ms unsure 🔍
-1% - +1%
-0.90ms - +1.48ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 1956.33ms - 1959.26ms - unsure 🔍
-0% - +0%
-1.84ms - +2.34ms
branch 536 kB 1956.05ms - 1959.03ms unsure 🔍
-0% - +0%
-2.34ms - +1.84ms
-

tags permalink

Version Bytes Avg Time vs remote vs branch
npm latest 324 kB 33.47ms - 33.82ms - unsure 🔍
-3% - +0%
-1.17ms - +0.12ms
branch 325 kB 33.54ms - 34.79ms unsure 🔍
-0% - +3%
-0.12ms - +1.17ms
-

toast permalink

Version Bytes Avg Time vs remote vs branch
npm latest 296 kB 49.55ms - 50.09ms - unsure 🔍
-2% - +1%
-0.85ms - +0.45ms
branch 298 kB 49.42ms - 50.61ms unsure 🔍
-1% - +2%
-0.45ms - +0.85ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 38.07ms - 38.47ms - unsure 🔍
-1% - +0%
-0.44ms - +0.10ms
branch 237 kB 38.26ms - 38.62ms unsure 🔍
-0% - +1%
-0.10ms - +0.44ms
-
Firefox

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 129.49ms - 142.11ms - unsure 🔍
-8% - +4%
-10.78ms - +5.74ms
branch 234 kB 132.99ms - 143.65ms unsure 🔍
-4% - +8%
-5.74ms - +10.78ms
-

action-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 400 kB 136.25ms - 152.35ms - unsure 🔍
-6% - +8%
-8.04ms - +11.88ms
branch 402 kB 136.51ms - 148.25ms unsure 🔍
-8% - +6%
-11.88ms - +8.04ms
-

action-group permalink

Version Bytes Avg Time vs remote vs branch
npm latest 426 kB 356.15ms - 368.69ms - unsure 🔍
-4% - +1%
-13.28ms - +4.52ms
branch 428 kB 360.49ms - 373.11ms unsure 🔍
-1% - +4%
-4.52ms - +13.28ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 459.93ms - 466.75ms - faster ✔
2% - 4%
7.34ms - 17.90ms
branch 597 kB 471.93ms - 479.99ms slower ❌
2% - 4%
7.34ms - 17.90ms
-

button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 321 kB 126.75ms - 133.93ms - unsure 🔍
-7% - +1%
-10.20ms - +1.12ms
branch 323 kB 130.50ms - 139.26ms unsure 🔍
-1% - +8%
-1.12ms - +10.20ms
-

card permalink

Version Bytes Avg Time vs remote vs branch
npm latest 379 kB 216.38ms - 230.02ms - unsure 🔍
-5% - +2%
-12.08ms - +5.60ms
branch 381 kB 220.82ms - 232.06ms unsure 🔍
-3% - +5%
-5.60ms - +12.08ms
-

dialog permalink

Version Bytes Avg Time vs remote vs branch
npm latest 306 kB 178.81ms - 191.15ms - unsure 🔍
-7% - +3%
-12.91ms - +4.83ms
branch 307 kB 182.64ms - 195.40ms unsure 🔍
-3% - +7%
-4.83ms - +12.91ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 297 kB 627.24ms - 639.60ms - unsure 🔍
-1% - +2%
-5.99ms - +9.87ms
branch 299 kB 626.52ms - 636.44ms unsure 🔍
-2% - +1%
-9.87ms - +5.99ms
-

number-field permalink

Version Bytes Avg Time vs remote vs branch
npm latest 492 kB 475.30ms - 483.74ms - faster ✔
0% - 3%
0.77ms - 12.19ms
branch 494 kB 482.16ms - 489.84ms slower ❌
0% - 3%
0.77ms - 12.19ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 208.20ms - 225.44ms - unsure 🔍
-2% - +8%
-4.18ms - +17.22ms
branch 337 kB 203.96ms - 216.64ms unsure 🔍
-8% - +2%
-17.22ms - +4.18ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 1303.45ms - 1321.87ms - unsure 🔍
-1% - +1%
-18.41ms - +6.81ms
branch 443 kB 1309.84ms - 1327.08ms unsure 🔍
-1% - +1%
-6.81ms - +18.41ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 119.71ms - 137.85ms - slower ❌
2% - 25%
2.69ms - 27.35ms
branch 231 kB 105.41ms - 122.11ms faster ✔
3% - 21%
2.69ms - 27.35ms
-

search permalink

Version Bytes Avg Time vs remote vs branch
npm latest 329 kB 236.54ms - 246.86ms - unsure 🔍
-3% - +2%
-7.51ms - +5.99ms
branch 331 kB 238.10ms - 246.82ms unsure 🔍
-2% - +3%
-5.99ms - +7.51ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 330 kB 402.52ms - 414.40ms - unsure 🔍
-3% - +1%
-14.20ms - +2.72ms
branch 331 kB 408.17ms - 420.23ms unsure 🔍
-1% - +3%
-2.72ms - +14.20ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 2118.72ms - 2132.96ms - unsure 🔍
-1% - +0%
-23.34ms - +2.26ms
branch 536 kB 2125.75ms - 2147.01ms unsure 🔍
-0% - +1%
-2.26ms - +23.34ms
-

tags permalink

Version Bytes Avg Time vs remote vs branch
npm latest 324 kB 76.03ms - 85.37ms - unsure 🔍
-9% - +6%
-7.72ms - +5.20ms
branch 325 kB 77.50ms - 86.42ms unsure 🔍
-7% - +10%
-5.20ms - +7.72ms
-

toast permalink

Version Bytes Avg Time vs remote vs branch
npm latest 296 kB 150.15ms - 166.73ms - unsure 🔍
-9% - +5%
-14.45ms - +7.81ms
branch 298 kB 154.33ms - 169.19ms unsure 🔍
-5% - +9%
-7.81ms - +14.45ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 91.39ms - 103.77ms - unsure 🔍
-7% - +8%
-7.04ms - +8.20ms
branch 237 kB 92.56ms - 101.44ms unsure 🔍
-8% - +7%
-8.20ms - +7.04ms
-

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.

2 participants