packages/core/src/view/handler/TooltipHandler.ts (3)

44-74: LGTM! Consider adding error handling for document.body check.

The initialization logic has been properly moved out of the constructor, aligning with the PR objectives. The event listeners are well-structured and handle edge cases appropriately.

Consider adding error handling for cases where document.body is null:

 init() {
   if (document.body != null) {
     this.div = document.createElement('div');
     // ... rest of the implementation
+  } else {
+    console.warn('TooltipHandler: Document body not available for tooltip initialization');
   }
 }

---

302-317: Consider improving null safety in the show method.

While the initialization flow is correct, the code assumes init() always succeeds and uses null assertions (!) extensively. This could be made more robust.

Consider this safer implementation:

 if (this.div == null) {
   this.init();
 }
-this.div!.style.zIndex = String(this.zIndex);
-this.div!.style.left = `${x + origin.x}px`;
-this.div!.style.top = `${y + TOOLTIP_VERTICAL_OFFSET + origin.y}px`;
+if (this.div) {
+  this.div.style.zIndex = String(this.zIndex);
+  this.div.style.left = `${x + origin.x}px`;
+  this.div.style.top = `${y + TOOLTIP_VERTICAL_OFFSET + origin.y}px`;

   if (!isNode(tip)) {
-    this.div!.innerHTML = (tip as string).replace(/\n/g, '<br>');
+    this.div.innerHTML = (tip as string).replace(/\n/g, '<br>');
   } else {
-    this.div!.innerHTML = '';
-    this.div!.appendChild(tip as HTMLElement);
+    this.div.innerHTML = '';
+    this.div.appendChild(tip as HTMLElement);
   }

-  this.div!.style.visibility = '';
-  fit(this.div!);
+  this.div.style.visibility = '';
+  fit(this.div);
+} else {
+  console.warn('TooltipHandler: Failed to initialize tooltip div');
+}

---

199-199: Fix indentation for better readability.

Add two spaces of indentation to align with the code style.

-              (me.isSource(state.shape) || !me.isSource(state.text)))))
+                (me.isSource(state.shape) || !me.isSource(state.text)))))

🧰 Tools

🪛 eslint

[error] 199-199: Insert ··

(prettier/prettier)

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check for any other potential tooltip div creation paths
# and verify that the tooltip div is only created in the init method

# Search for div creation
echo "Checking for div creation patterns:"
rg "createElement.*div" --type ts

# Search for mxTooltip class usage
echo "Checking for mxTooltip class usage:"
rg "className.*mxTooltip" --type ts

# Search for tooltip initialization patterns
echo "Checking for tooltip initialization patterns:"
rg "new TooltipHandler" --type ts -A 5

#!/bin/bash
# Let's verify the tooltip initialization with a more focused search

# Check for TooltipHandler instantiation and usage
echo "Checking TooltipHandler instantiation and usage:"
rg "TooltipHandler" --type ts -B 2 -A 2

# Check for any direct div access in TooltipHandler
echo "Checking div access patterns in TooltipHandler:"
ast-grep --pattern 'this.div' packages/core/src/view/handler/TooltipHandler.ts

# Check for any tooltip-related plugin registration
echo "Checking tooltip plugin registration:"
rg "TooltipHandler.pluginId" --type ts

packages/core/src/view/handler/TooltipHandler.ts (3)

44-74: Consider handling document.body availability more robustly

The implementation correctly moves tooltip creation to a lazy-loaded approach. However, there's a potential edge case where document.body might not be available when init() is called.

Consider using a more robust approach:

 init() {
-  if (document.body != null) {
+  if (!document.body) {
+    // Wait for document.body to be available
+    setTimeout(() => this.init(), 0);
+    return;
+  }
   this.div = document.createElement('div');
   // ... rest of the implementation
 }

---

302-317: Consider more robust null handling

While the initialization check is good, the subsequent non-null assertions (this.div!) could be handled more elegantly to maintain type safety.

Consider refactoring to avoid multiple non-null assertions:

 show(tip: HTMLElement | string | null, x: number, y: number) {
   if (!this.destroyed && tip && tip !== '') {
     const origin = getScrollOrigin();
     if (this.div == null) {
       this.init();
     }
+    if (this.div == null) {
+      console.warn('Failed to initialize tooltip div');
+      return;
+    }
-    this.div!.style.zIndex = String(this.zIndex);
-    this.div!.style.left = `${x + origin.x}px`;
+    const div = this.div;  // Create a local reference
+    div.style.zIndex = String(this.zIndex);
+    div.style.left = `${x + origin.x}px`;
     // ... rest of the method using div instead of this.div!
   }
 }

---

Line range hint 1-341: Consider adding tests for lazy initialization

The changes successfully implement lazy loading of the tooltip div, but we should ensure this behavior is covered by tests.

Would you like me to help create test cases that verify:

1. Tooltip div is not created on initialization
2. Tooltip div is created only when show() is called
3. Proper cleanup in various scenarios

Configuration used: CodeRabbit UI
Review profile: CHILL

packages/core/src/view/handler/TooltipHandler.ts (2)

44-74: Ensure event cleanup for 'mouseleave' on container

The lazy creation of the tooltip div is well done to only instantiate when needed. However, consider removing the 'mouseleave' listener from the graph container upon destruction. This helps prevent memory leaks if the container remains in the DOM after the tooltip handler has been destroyed.

---

289-292: Avoid clearing innerHTML if other child nodes must persist

Clearing .innerHTML is generally fine for a pure text tooltip. However, if the tooltip could at times contain other nodes, ensure that removing them is indeed the desired behavior. Using .textContent = '' might be safer for text-only contexts.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro