From e05d97399b99a034adc77b6ee9d785ddd2d49897 Mon Sep 17 00:00:00 2001
From: ankit mishra <mishra.ankit.learn@outlook.com>
Date: Fri, 22 Mar 2024 21:53:38 +0530
Subject: [PATCH 1/2] #1224 | useSref | updated onClick to check target from
 currentProperty rather than from target so that browser's onClick can be used
 in cases where anchor has a child

---
 src/hooks/__tests__/useSref.test.tsx | 19 +++++++++++++++++++
 src/hooks/useSref.ts                 |  8 +++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/hooks/__tests__/useSref.test.tsx b/src/hooks/__tests__/useSref.test.tsx
index 8d8f490e..4f904714 100644
--- a/src/hooks/__tests__/useSref.test.tsx
+++ b/src/hooks/__tests__/useSref.test.tsx
@@ -20,6 +20,15 @@ const Link = ({ to, params = undefined, children = undefined, target = undefined
   );
 };
 
+const LinkWithChildElement = ({ to, params = undefined, children = undefined, target = undefined }) => {
+  const sref = useSref(to, params);
+  return (
+    <a data-testid="anchor" {...sref} target={target}>
+      <div data-testid="inner-child">{children}</div>
+    </a>
+  );
+};
+
 describe('useSref', () => {
   let { router, routerGo, mountInRouter } = makeTestRouter([]);
   beforeEach(() => ({ router, routerGo, mountInRouter } = makeTestRouter([state, state2, state3])));
@@ -83,6 +92,16 @@ describe('useSref', () => {
       rendered.getByTestId('anchor').click();
       expect(spy).not.toHaveBeenCalled();
     });
+
+    it('does not get called if the underlying DOM element that has a child has a "target" attribute', () => {
+      const spy = jest.spyOn(router.stateService, 'go');
+      const rendered = mountInRouter(<LinkWithChildElement to="state" target="_blank" />);
+
+      // clicking on the div to mimick the actual behaviour
+      // behaviour: when anchor has a child element and user clicks on anchor, the event is fired on child element rather than on anchor
+      rendered.getByTestId('inner-child').click();
+      expect(spy).not.toHaveBeenCalled();
+    });
   });
 
   it('updates the href when the stateName changes', () => {
diff --git a/src/hooks/useSref.ts b/src/hooks/useSref.ts
index 96fcd04e..daa0a055 100644
--- a/src/hooks/useSref.ts
+++ b/src/hooks/useSref.ts
@@ -90,7 +90,13 @@ export function useSref(stateName: string, params: object = {}, options: Transit
 
   const onClick = useCallback(
     (e: React.MouseEvent) => {
-      const targetAttr = (e.target as any)?.attributes?.target;
+      /* For scenario where we specify target="_blank" on a link that has a div as a child, to use browser's default onClick behavior,
+          the onClick event gets triggered on the div rather than the anchor tag.
+          The actual target is present inside the currentTarget property instead. So, targetAttr should be assigned from currentTarget,
+          so that router's go method does not get called and browser's default onClick behavior is triggered 
+          currentTarget will always be set irrespective of anchor contains a child or not*/
+
+      const targetAttr = (e.currentTarget as any)?.attributes?.target;
       const modifierKey = e.button >= 1 || e.ctrlKey || e.metaKey || e.shiftKey || e.altKey;
       if (!e.defaultPrevented && targetAttr == null && !modifierKey) {
         e.preventDefault();

From dceb1581daefb64f06c1a9ae61f96ee8218c77cb Mon Sep 17 00:00:00 2001
From: ankit mishra <mishra.ankit.learn@outlook.com>
Date: Fri, 22 Mar 2024 22:27:00 +0530
Subject: [PATCH 2/2] #1224 - updated the comment to be more precise

---
 src/hooks/useSref.ts | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/hooks/useSref.ts b/src/hooks/useSref.ts
index daa0a055..038808bb 100644
--- a/src/hooks/useSref.ts
+++ b/src/hooks/useSref.ts
@@ -90,11 +90,7 @@ export function useSref(stateName: string, params: object = {}, options: Transit
 
   const onClick = useCallback(
     (e: React.MouseEvent) => {
-      /* For scenario where we specify target="_blank" on a link that has a div as a child, to use browser's default onClick behavior,
-          the onClick event gets triggered on the div rather than the anchor tag.
-          The actual target is present inside the currentTarget property instead. So, targetAttr should be assigned from currentTarget,
-          so that router's go method does not get called and browser's default onClick behavior is triggered 
-          currentTarget will always be set irrespective of anchor contains a child or not*/
+      /* We want to check attributes on the element that the click handler is on and not on whichever other element was clicked*/
 
       const targetAttr = (e.currentTarget as any)?.attributes?.target;
       const modifierKey = e.button >= 1 || e.ctrlKey || e.metaKey || e.shiftKey || e.altKey;