#9338 Add link toolbar action #9445
Conversation
This comment has been minimized.
This comment has been minimized.
Playwright test results
Details
Flaky testsmsedge › tests/modLifecycle.spec.ts › create, run, package, and update mod Skipped testschrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9445 +/- ##
==========================================
+ Coverage 74.24% 75.79% +1.54%
==========================================
Files 1332 1421 +89
Lines 40817 42907 +2090
Branches 7634 7899 +265
==========================================
+ Hits 30306 32522 +2216
+ Misses 10511 10385 -126 ☔ View full report in Codecov by Sentry. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…feature/9338_add_link_toolbar_action
| }, []); | ||
|
|
||
| const handleHide = useCallback((event: Event) => { | ||
| // Check if the click path includes our button |
There was a problem hiding this comment.
huge nit: an isButtonClick variable name would satisfy the the need to comment
| }, [editor]); | ||
|
|
||
| const handleHide = useCallback((event: Event) => { | ||
| // Check if the click path includes our button |
There was a problem hiding this comment.
nit: see other comment about the need to comment here. If we find ourselves using this pattern one more time I'm inclined to extract it since it's handling a non-intuitive corner case
| popoverView: PopoverState["popoverView"]; | ||
| }) => { | ||
| const { editor } = useCurrentEditor(); | ||
| assertNotNullish(editor, "Tiptap editor must be in scope"); |
There was a problem hiding this comment.
❤️ I was able to confirm btw by digging into the implementation that this seems to be the only reason why editor would be null; we can probably replace our other checks with this
...ns/browser-extension/src/components/richTextEditor/toolbar/LinkButton/LinkButton.module.scss
Outdated
Show resolved
Hide resolved
|
When the PR is merged, the first loom link found on this PR will be posted to |
What does this PR do?
Discussion
Demo
https://www.loom.com/share/38a87c6024544d57bb7f879df4dae6cb