packages/html/stories/Events.stories.ts (2)

61-61: Consider using a more specific type definition.

The Record<string, string> type annotation suggests all parameters are strings, but some arguments (like contextMenu and rubberBand) are likely booleans based on their usage in the code.

-const Template = ({ label, ...args }: Record<string, string>) => {
+const Template = ({ label, ...args }: { 
+  label?: string;
+  contextMenu?: boolean;
+  rubberBand?: boolean;
+  [key: string]: any;
+}) => {

---

155-175: Consider replacing alerts with a more modern notification approach.

Using alert() for notifications is generally not recommended in modern web applications as it blocks the UI thread and provides a poor user experience.

Consider replacing the alerts with a more modern notification system that is non-blocking, such as toast notifications or a custom feedback component within the Storybook interface:

-    menu.addItem('Cell Item', `${Client.imageBasePath}/image.gif`, () => {
-      alert('MenuItem1');
-    });
+    menu.addItem('Cell Item', `${Client.imageBasePath}/image.gif`, () => {
+      // Use a non-blocking notification or logging for Storybook
+      console.log('MenuItem1 clicked');
+      // Alternatively, update a status element in the DOM
+      const statusEl = document.getElementById('status-message') || document.createElement('div');
+      statusEl.id = 'status-message';
+      statusEl.textContent = 'MenuItem1 clicked';
+      statusEl.style.cssText = 'position: fixed; bottom: 10px; right: 10px; background: #333; color: white; padding: 8px; border-radius: 4px;';
+      document.body.appendChild(statusEl);
+      setTimeout(() => statusEl.remove(), 3000);
+    });

Apply similar changes to the other alert calls.

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

packages/html/stories/Events.stories.ts (4)

61-61: Consider using a more specific type for the Template function parameters.

The current type Record<string, string> is too restrictive as some arguments might be booleans or numbers coming from Storybook controls.

-const Template = ({ label, ...args }: Record<string, string>) => {
+const Template = ({ label, ...args }: {label?: string, [key: string]: any}) => {

---

64-76: Consider extracting the description to a separate component or constant.

The current implementation directly manipulates DOM elements and sets inline HTML, which works but isn't the most maintainable approach. Consider:

1. Extracting this to a separate component
2. Or at minimum, moving the HTML content to a constant

This would improve code organization and readability.

+const DESCRIPTION_HTML = `
+  Creating a graph container and using interaction on the graph, including marquee selection, custom tooltips, context menu.
+  <p>
+  It also demonstrates how to use an edge style in the default stylesheet, and handle the double click on the adjustment point.
+  <br>
+   See also the <code>Overlays</code> Story for click event handling.
+  `;

const Template = ({ label, ...args }: Record<string, string>) => {
  configureImagesBasePath();

  const div = document.createElement('div');

  const divMessage = document.createElement('div');
-  divMessage.innerHTML = `
-  Creating a graph container and using interaction on the graph, including marquee selection, custom tooltips, context menu.
-  <p>
-  It also demonstrates how to use an edge style in the default stylesheet, and handle the double click on the adjustment point.
-  <br>
-   See also the <code>Overlays</code> Story for click event handling.
-  `;
+  divMessage.innerHTML = DESCRIPTION_HTML;
  divMessage.style.fontFamily = 'Arial,Helvetica,sans-serif';
  divMessage.style.marginBottom = '1rem';
  div.appendChild(divMessage);

---

143-150: Consider using more descriptive variable names for vertices.

The variables v1, v2, v3 don't convey the purpose of each vertex. Using more descriptive names would improve code readability and maintainability.

-    const v1 = graph.insertVertex(parent, null, 'Double-click', 20, 20, 80, 30);
-    const v2 = graph.insertVertex(parent, null, 'Right-/Shift-click', 200, 150, 120, 30);
-    const v3 = graph.insertVertex(parent, null, 'Connect/Reconnect', 200, 20, 120, 30);
+    const doubleClickVertex = graph.insertVertex(parent, null, 'Double-click', 20, 20, 80, 30);
+    const rightShiftClickVertex = graph.insertVertex(parent, null, 'Right-/Shift-click', 200, 150, 120, 30);
+    const connectReconnectVertex = graph.insertVertex(parent, null, 'Connect/Reconnect', 200, 20, 120, 30);
     graph.insertVertex(parent, null, 'Control-Drag', 20, 150, 100, 30);
-    graph.insertEdge(parent, null, 'Tooltips', v1, v2);
-    graph.insertEdge(parent, null, '', v2, v3);
+    graph.insertEdge(parent, null, 'Tooltips', doubleClickVertex, rightShiftClickVertex);
+    graph.insertEdge(parent, null, '', rightShiftClickVertex, connectReconnectVertex);

---

155-175: Replace alert() with a more user-friendly notification mechanism.

Using alert() for notifications in the context menu handlers is functional but disruptive to the user experience. Consider using a more modern notification approach like toast messages or an overlay that doesn't block the UI.

function createPopupMenu(
  graph: Graph,
  menu: MaxPopupMenu,
  cell: Cell | null,
  _evt: MouseEvent
) {
  // Function to create the entries in the popupmenu
  if (cell != null) {
    menu.addItem('Cell Item', `${Client.imageBasePath}/image.gif`, () => {
-     alert('MenuItem1');
+     // Consider a non-blocking notification approach
+     showNotification('MenuItem1 selected');
    });
  } else {
    menu.addItem('No-Cell Item', `${Client.imageBasePath}/image.gif`, () => {
-     alert('MenuItem2');
+     showNotification('MenuItem2 selected');
    });
  }
  menu.addSeparator();
  menu.addItem('MenuItem3', `${Client.imageBasePath}/warning.gif`, () => {
-   alert(`MenuItem3: ${graph.getSelectionCount()} selected`);
+   showNotification(`MenuItem3: ${graph.getSelectionCount()} selected`);
  });
}

+// Example implementation of a non-blocking notification
+function showNotification(message: string) {
+  const notification = document.createElement('div');
+  notification.textContent = message;
+  notification.style.position = 'fixed';
+  notification.style.bottom = '20px';
+  notification.style.right = '20px';
+  notification.style.padding = '10px 20px';
+  notification.style.backgroundColor = '#333';
+  notification.style.color = 'white';
+  notification.style.borderRadius = '4px';
+  notification.style.zIndex = '1000';
+  document.body.appendChild(notification);
+  
+  setTimeout(() => {
+    notification.style.opacity = '0';
+    notification.style.transition = 'opacity 0.5s ease';
+    setTimeout(() => document.body.removeChild(notification), 500);
+  }, 3000);
+}

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