fix(overlay): only "tab trap" when you mean to #885
Conversation
| @@ -90,6 +90,11 @@ export class OverlayStack { | |||
| } | |||
|
|
|||
| private startTabTrapping(): void { | |||
| if (!this.trappingInited) { | |||
| this.initTabTrapping(); | |||
There was a problem hiding this comment.
It might work a little better to have a disableTabTrapping option. I'd rather not cause all clients to do extra work if they're not running into the Safari issue.
There was a problem hiding this comment.
Other than the boolean check, I don't particularly count this as extra work. I had previously been making this call in the constructor() which means if an application doesn't use tab trapping this change now means they do less work.
A universal disabledTabTrapping options is interesting, however. Where do you think you'd want to apply that? Currently, there are no packages that leverage tab trapping on their own (IIRC), so an implementing app could also not use type="modal" (or manage their own usage of that value) and this would never be needed in that application.
This does point to the benefit of a more durable solution. To prevent such a solution from adding work to non-Safari browsers, what are you thoughts on in component browser checking?
There was a problem hiding this comment.
Oh never mind -- I just misunderstood this logic. I'm not even sure how 🙈. This looks perfect.
Westbrook
force-pushed
the
westbrook/overlay-safari
branch
from
01a7445
to
aede0db
Compare
Description
Safari has issues with WebGL canvas in association with out tab trapping solution. This mitigated the issue by initializing the tab trapping approach only when tab trapping is being used by an element.
I have another idea that is low touch that I want to try, but all of my other ideas in this area haven't been working, so having something proposed as a possible path forward feels nice...
Related Issue
refs #848
Motivation and Context
You should be able to have WebGL canvas AND use SWC.
How Has This Been Tested?
Types of changes
Checklist: