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

With Shoelace version 2.6.0+ tabbing crashes browser. #1612

Closed
Jaylyn-Barbee opened this issue Oct 12, 2023 · 3 comments · Fixed by #1614
Closed

With Shoelace version 2.6.0+ tabbing crashes browser. #1612

Jaylyn-Barbee opened this issue Oct 12, 2023 · 3 comments · Fixed by #1614
Assignees
@KonnorRogers
Labels
bug Things that aren't working right in the library.

Comments

@Jaylyn-Barbee
Copy link

Jaylyn-Barbee commented Oct 12, 2023 •
edited
Loading

Describe the bug

A bug is a demonstrable problem caused by code in the library. Please provide a clear and concise description of what the bug is here.

We have sl-tab-group inside of sl-dialog and when we upgraded to version 2.9.0 my browser stopped working when trying to tab.

To Reproduce

Steps to reproduce the behavior:
Note: 2.9.0 is where this happens the worst but the environment below is on 2.7.0 because its at least usable for us

  1. Go to https://preview.pwabuilder.com
  2. Click on "demo_url" under the input box
  3. Wait for load and click "Manifest Editor"
  4. Try to tab
  5. If you record in the performance tab of devtools, you can see tons of calls to getTabbableElements which then calls walk a bunch.

Demo

I attached the performance file from DevTools

Screenshots

If applicable, add screenshots to help explain the bug.

Browser / OS

  • OS: Windows 11
  • Browser: Edge
  • Browser version: Latest

Additional information

Provide any additional information about the bug here.
Shoelace version 2.9.0 is what we were using in the file below.
Trace-20231012T143246.json
@Jaylyn-Barbee Jaylyn-Barbee added the bug Things that aren't working right in the library. label Oct 12, 2023
@Jaylyn-Barbee Jaylyn-Barbee mentioned this issue Oct 12, 2023
Open
@KonnorRogers
Copy link
Collaborator

@Jaylyn-Barbee Thanks for the report. So I maanged to recreate a similar-ish page to the one in the PWA manifest and can recreate the slow down in there.

The slowdown seems to only be Chrome / Edge specific, and it appears to be coming from our composed-offset-position ponyfill for a browser behavior that got yoinked for detecting if an element is visible.

https://github.com/jcfranco/composed-offset-position

@KonnorRogers
Copy link
Collaborator

KonnorRogers commented Oct 12, 2023 •
edited
Loading

Reproduction of the issue here:

(i didnt even use extra shadow roots either, which probably causes even more slowness)

https://codepen.io/paramagicdev/pen/rNobeqE?editors=1000

I crudely copied the code from the initial manifest page. (Hope thats okay)

@Jaylyn-Barbee
Copy link
Author

No worries on reproducing out code, we're open source after all.

@KonnorRogers KonnorRogers mentioned this issue Oct 12, 2023
Merged
@Jaylyn-Barbee Jaylyn-Barbee mentioned this issue Oct 16, 2023
Merged
3 tasks
github-merge-queue bot pushed a commit to pwa-builder/PWABuilder that referenced this issue Oct 16, 2023
@Jaylyn-Barbee @jgw96
@khmyznikov @nmetulev @maraah1 @dependabot @adolfdaniel @zateutsch @beth-panx @vipul-bhojwani @amrutha95 @FANMixco @ybot1122 @microsoft-github-policy-service @dargmuesli @SirajChokshi
Main <-- Staging (#4456)
a26ff48 
fixes/features (should be multiple, add to the list)
1. App Capabilities:
#4034
2. Accessibility issues: #4258, #4259, #4239, #4240

## Describe the current behavior?
<!-- Please describe the current behavior that is being modified or link
to a relevant issue. -->
1. App Capabilities are mixed in with manifest and SW fields and there
is no easy way to discover and learn about them
2. Accessibility Issues from most recent pass

## Describe the new behavior?
1. New designs to isolate app capabilities and give our users more
information about what they are and how to implement them.
2. Fixed those issues

## PR Checklist
- [x] Test: run `npm run test` and ensure that all tests pass
- [x] Target main branch (or an appropriate release branch if
appropriate for a bug fix)
- [x] Ensure that your contribution follows [standard accessibility
guidelines](https://docs.microsoft.com/en-us/microsoft-edge/accessibility/design).
Use tools like https://webhint.io/ to validate your changes.

## Additional Information
1. Known issue: #4453
waiting on dependency
(shoelace-style/shoelace#1612).
2. No additional context

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Justin Willis <jgw9617@gmail.com>
Co-authored-by: Gleb Khmyznikov <gleb.khmyznikov@gmail.com>
Co-authored-by: Nikola Metulev <nmetulev@users.noreply.github.com>
Co-authored-by: Mara'ah Lee <maraahlee@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Adolf Daniel <10156724+adolfdaniel@users.noreply.github.com>
Co-authored-by: Justin Willis (HE / HIM) <juwillis@microsoft.com>
Co-authored-by: Zach Teutsch <88554871+zateutsch@users.noreply.github.com>
Co-authored-by: Beth Pan <xupa@microsoft.com>
Co-authored-by: vipul-bhojwani <67650372+vipul-bhojwani@users.noreply.github.com>
Co-authored-by: Amrutha Srinivasan <amrutha.srinivasan95@gmail.com>
Co-authored-by: Federico Navarrete <darklord.navarrete@gmail.com>
Co-authored-by: Toby Liu <ybot1122@gmail.com>
Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.com>
Co-authored-by: Amrutha Srinivasan <amsrin@microsoft.com>
Co-authored-by: Jonas Thelemann <e-mail@jonas-thelemann.de>
Co-authored-by: Siraj Chokshi <19193347+SirajChokshi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@KonnorRogers KonnorRogers

Labels
bug Things that aren't working right in the library.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix tabbable performance issues in Chrome / Edge shoelace-style/shoelace
2 participants
@KonnorRogers @Jaylyn-Barbee