pixiebrix-extension icon indicating copy to clipboard operation
pixiebrix-extension copied to clipboard

#7670: Simplify injection of widgets onto the page (`renderWidget`)

Open fregante opened this issue 1 year ago • 1 comments

What does this PR do?

  • Part of https://github.com/pixiebrix/pixiebrix-extension/issues/7670
  • Complementary to #8139

This helper:

  • simplifies the injection of widgets onto the page
  • automatically removes the widget on context invalidated, and via supplied AbortSignal
  • ~~avoids the additional wrapper created outside React that cause regularly causes conflicts~~
    • unfortunately React 17 doesn't properly support this, so I left a TODO for React 18

I expect most isolated components to use both the IsolatedWidget component (#7670) and renderWidget:


renderWidget({
  name: "SelectionMenu", // Identification in the dev tools
  widget: 
    <IsolatedComponent name="SelectionMenu">
      etc
    </IsolatedComponent>
});

Future work

  • [ ] Migrate the rest of the ReactDOM.render() usage here, particularly the popover utils will have the most benefit complexity-wise

Checklist

  • [x] Add tests and/or storybook stories
  • [x] Designate a primary reviewer: @grahamlangford

fregante avatar Apr 04 '24 10:04 fregante

Codecov Report

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

Project coverage is 73.44%. Comparing base (1995795) to head (c82c591).

:exclamation: Current head c82c591 differs from pull request most recent head 88e004b. Consider uploading reports for the commit 88e004b to get more accurate results

Files Patch % Lines
src/contentScript/modalDom.tsx 45.45% 6 Missing :warning:
src/utils/reactUtils.tsx 80.00% 4 Missing :warning:
src/hooks/useContextInvalidated.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8150      +/-   ##
==========================================
+ Coverage   73.37%   73.44%   +0.06%     
==========================================
  Files        1314     1316       +2     
  Lines       40722    40735      +13     
  Branches     7571     7575       +4     
==========================================
+ Hits        29879    29916      +37     
+ Misses      10843    10819      -24     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 04 '24 10:04 codecov[bot]

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

github-actions[bot] avatar Apr 08 '24 14:04 github-actions[bot]

Tested again, it works.

However I realized that we want notifications to survive context invalidation since we use them to inform the user:

https://github.com/pixiebrix/pixiebrix-extension/blob/88e004b47869fbff2491351c86a7f2b8729a4f13/src/errors/contextInvalidated.ts#L29-L33

So I added a keepAfterInvalidation option specifically for this case.

fregante avatar Apr 08 '24 15:04 fregante