#6904: Stop propagation of certain events in QuickBar

[fregante]

What does this PR do?

• Fixes QuickBar - focus lock not working on Notion #6904

Demo

Screen.Recording.2.mov

Checklist

• Add tests
• New files added to src/tsconfig.strictNullChecks.json (if possible)
• Designate a primary reviewer: @grahamlangford

[fregante]

This shadow root is open btw, I don’t think it's intentional:

[twschiller]

Hmm, it was switched from closed to open when we switched libraries: https://github.com/pixiebrix/pixiebrix-extension/pull/5750/files#diff-50f2a63c2398eca75310e434d2dc90a762b23dd6591766e05b68d38a5e00c3a8L122. That wasn't flagged during that PR

I agree it should likely be closed - host sites shouldn't need to snoop on the individual events within the Quick Bar

[fregante]

I closed it now 👌

[twschiller]

@grahamlangford this change will need to be tested on Salesforce, Pipedrive, Zendesk and other platforms supporting hotkeys

@fregante what's the reasoning behind dropping focus lock? Any additional investigation beyond the issue?: #6904 (comment). From my original commit, it seems like it was added because some apps might have tried stealing focus away (I don't remember if that was in response to hotkey or not)?: #5173

[fregante]

I looked at the original implementation and it mentioned "stealing key presses" rather than focus. The focus change was probably a consequence of the key press going through.

[fregante]

The QuickBar seems to work correctly even without it, automatically catching the keypresses and restoring focus. Here I had a setInterval that focused the first input on the page, and no keypresses were lost:

Screen.Recording.9.mov

The focus lock component might be useful in the future, but it seems that kbar + StopPropagation handles this scenario

[fregante]

I created a playground here: https://pbx.vercel.app/site-with-hotkeys/

As mentioned in the issue, we can't really prevent the document from seeing the events if they use capture:

I'll add an input blocker too

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 72.55%. Comparing base (a8d3c3b) to head (8e26ff6).
Report is 3 commits behind head on main.

Files	Patch %	Lines
src/components/StopPropagation.tsx	84.61%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7686      +/-   ##
==========================================
+ Coverage   72.54%   72.55%   +0.01%     
==========================================
  Files        1259     1260       +1     
  Lines       39264    39277      +13     
  Branches     7326     7328       +2     
==========================================
+ Hits        28484    28498      +14     
+ Misses      10780    10779       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[twschiller]

I looked at the original implementation and it mentioned "stealing key presses" rather than focus. The focus change was probably a consequence of the key press going through.

I'll have to try this on Salesforce, etc. IIRC, the problem was that some sites have key event handlers on document with capture: true. And, because the event handlers added as part of the JS, their event handlers run first. So they can handle/stop the event before our quick bar sees the event.

The [contenteditable] and cke_editable hacks might be sufficient though, so might be able to drop the focus lock. I think it's worth giving it a try.