fix: fix the issue where the pop-up box for modification operations does not appear when hovering over an image link#463
fix: fix the issue where the pop-up box for modification operations does not appear when hovering over an image link#463wuyiping0628 wants to merge 1 commit intodevfrom
Conversation
…oes not appear when hovering over an image link
WalkthroughUpdated tooltip handling logic to differentiate event target types: when hovering over image elements, the parent node is used for link format detection instead of the image itself. Added debug logging to the edit method. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/fluent-editor/src/modules/link/modules/tooltip.ts (1)
98-100: Use safe target narrowing and anchor resolution viaclosest()instead ofparentNode.Line 98 currently special-cases only direct
IMG -> parentNode. That misses nested structures and relies on unsafeEventTargetproperty access. Resolve the anchor directly and guard null.♻️ Proposed adjustment
- const linkNode = event.target.tagName === 'IMG' - ? event.target.parentNode as HTMLElement - : event.target as HTMLElement + const target = event.target as HTMLElement | null + const linkNode = target?.closest(`a.${LinkBlot.className}`) as HTMLElement | null + if (!linkNode) { + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts` around lines 98 - 100, The code currently narrows event.target unsafely and special-cases an IMG -> parentNode path to compute linkNode; change this to safely check that event.target is an Element (using instanceof Element) and resolve the anchor using Element.closest('a') so nested structures are handled and null is guarded; update the logic around the linkNode variable in tooltip.ts (where event.target is used) to use the closest('a') result and bail out if it's null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts`:
- Around line 316-317: Remove the leftover debug console.log from the edit()
function in tooltip.ts: delete the line console.log('1111 edit') (which violates
the no-console lint rule) and if you need to keep a message for debugging
replace it with the project's logger/debug utility (e.g., logger.debug or
similar) or remove it entirely before merging.
---
Nitpick comments:
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts`:
- Around line 98-100: The code currently narrows event.target unsafely and
special-cases an IMG -> parentNode path to compute linkNode; change this to
safely check that event.target is an Element (using instanceof Element) and
resolve the anchor using Element.closest('a') so nested structures are handled
and null is guarded; update the logic around the linkNode variable in tooltip.ts
(where event.target is used) to use the closest('a') result and bail out if it's
null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 316ce74d-7530-4353-acc5-0b44a9f0c929
📒 Files selected for processing (1)
packages/fluent-editor/src/modules/link/modules/tooltip.ts
| console.log('1111 edit') | ||
|
|
There was a problem hiding this comment.
Remove debug console.log from edit() before merge.
Line 316 violates the current lint rule (no-console) and looks like leftover debug output.
🧹 Proposed fix
- console.log('1111 edit')
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('1111 edit') |
🧰 Tools
🪛 ESLint
[error] 316-316: Unexpected console statement. Only these console methods are allowed: warn, error.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts` around lines 316
- 317, Remove the leftover debug console.log from the edit() function in
tooltip.ts: delete the line console.log('1111 edit') (which violates the
no-console lint rule) and if you need to keep a message for debugging replace it
with the project's logger/debug utility (e.g., logger.debug or similar) or
remove it entirely before merging.
…oes not appear when hovering over an image link
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit