feat: add multiplet signal in 1d

[hamed-musallam]

No description provided.

[cloudflare-workers-and-pages]

Deploying nmrium with    Cloudflare Pages

Latest commit:	b83f147
Status:	✅  Deploy successful!
Preview URL:	https://bae857de.nmrium.pages.dev
Branch Preview URL:	https://add-signal-manual-1d.nmrium.pages.dev

View logs

[lpatiny]

Functionality looks great, thanks !

Would it be possible to visualize that we are in the addition mode of a signal ?
I'm thinking for example that when we are hover the zone there is a pink ball that moves to highlight that if ever we click we will add a new signal. Could be another issue if you prefer.

[hamed-musallam]

Functionality looks great, thanks !

Would it be possible to visualize that we are in the addition mode of a signal ? I'm thinking for example that when we are hover the zone there is a pink ball that moves to highlight that if ever we click we will add a new signal. Could be another issue if you prefer.

Yes, I think it is a good idea.

[lpatiny]

Seems there is a bug removing a signal

TypeError: 'get' on proxy: property 'prototype' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '#' but got '[object Object]')

[lpatiny]

Currently it is also very easy to cut the range rather than adding a new signal. I think that depending the exact click position we can have both, add a signal and cut the range, which is problematic.

[lpatiny]

I see my issue. When I try to move a point I sometimes leave the horizontal line and then the range is being cut. Not sure if we can prevent a vertical mouvement or how to prevent this range cutting.

[lpatiny]

In fact I'm not able to move a signal without cutting the range

[hamed-musallam]

In fact I'm not able to move a signal without cutting the range

I see the issue. We need to stop propagating the click event to the anchor.

[hamed-musallam]

@targos

Could you take a look at my last commit? I added a workaround to prevent the click event from firing in BrushTrackerafter dragging the anchor. Do you know of a cleaner way to handle this?

[targos]

What about extending the data-self-control thing that you already added to the pointerdown event?

Subject: [PATCH] suggestion
---
Index: src/component/EventsTrackers/BrushTracker.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/component/EventsTrackers/BrushTracker.tsx b/src/component/EventsTrackers/BrushTracker.tsx
--- a/src/component/EventsTrackers/BrushTracker.tsx	(revision 6b535e0ce1aa790161409e70cb7b234718c266b4)
+++ b/src/component/EventsTrackers/BrushTracker.tsx	(date 1783500892603)
@@ -144,6 +144,10 @@
   return { x, y };
 }
 
+function isSelfControlledTarget(target: EventTarget) {
+  return (target as Element).closest('[data-self-control="true"]') !== null;
+}
+
 export function BrushTracker(options: BrushTrackerProps) {
   const {
     children,
@@ -199,13 +203,26 @@
   );
 
   function handleClick(event: MouseEvent) {
-    if (isDraggingRef.current || clickTriggerMode !== 'native') return;
+    if (
+      isDraggingRef.current ||
+      clickTriggerMode !== 'native' ||
+      isSelfControlledTarget(event.target)
+    ) {
+      return;
+    }
+
     const { x, y } = getMouseXY(event);
     onClick({ ...event, x, y });
   }
 
   function handleDoubleClick(event: MouseEvent) {
-    if (isDraggingRef.current || clickTriggerMode !== 'native') return;
+    if (
+      isDraggingRef.current ||
+      clickTriggerMode !== 'native' ||
+      isSelfControlledTarget(event.target)
+    ) {
+      return;
+    }
 
     const { x, y } = getMouseXY(event);
     onDoubleClick({ ...event, x, y });
@@ -216,8 +233,7 @@
       event.persist();
       const currentTarget = event.currentTarget;
 
-      const target = event.target as HTMLElement;
-      if (target.closest('[data-self-control="true"]')) {
+      if (isSelfControlledTarget(event.target)) {
         return;
       }
 
Index: src/component/AnchorSVG.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/component/AnchorSVG.tsx b/src/component/AnchorSVG.tsx
--- a/src/component/AnchorSVG.tsx	(revision 6b535e0ce1aa790161409e70cb7b234718c266b4)
+++ b/src/component/AnchorSVG.tsx	(date 1783500290624)
@@ -279,16 +279,6 @@
     };
 
     const onUp = () => {
-      if (dragState.current && stopClickPropagation) {
-        // Suppress the click event the browser fires after the drag ends.
-        function suppressClick(clickEvent: globalThis.MouseEvent) {
-          clickEvent.preventDefault();
-          clickEvent.stopPropagation();
-          window.removeEventListener('click', suppressClick, true);
-        }
-        window.addEventListener('click', suppressClick, true);
-      }
-
       setDragging(false);
       dragState.current = null;
       onDragEnd({ x: position.x, y: position.y });

[hamed-musallam]

Yes, I agree. I was trying to avoid a custom attribute, but you are right, we can extend the existing use of data-self-control instead.

[lpatiny]

Functionality looks perfect to me, thanks !

Would it still be possible to hide the red balls if the Range / Multiplet tool is not selected ?

A end-user would not like to have those balls in the pdf they generate for a publication or report.