[newswires] ensure 'add selection' buttons don't ever burst out of co…

…ntainer, by detecting left vs right and shifting the position accordingly
guardian · Dec 16, 2024 · 87ed411 · 87ed411
1 parent 0553e64
commit 87ed411
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 44 deletions.
diff --git a/bootstrapping-lambda/local/index.html b/bootstrapping-lambda/local/index.html
@@ -21,32 +21,6 @@
   </head>
 
   <body>
-    <hr />
-    <p>not inside target</p>
-    <h2>Not inside target</h2>
-    <div
-      data-pinboard-selection-target
-      style="padding: 20px; background-color: aliceblue"
-    >
-      <h3>Pinboard selection target</h3>
-      <p>
-        This is a target for the Pinboard library to render the selection
-        interface into. It will be hidden by the library when not in use. This
-        is a target for the Pinboard library to render the selection interface
-        into. It will be hidden by the library when not in use. This is a target
-        for the Pinboard library to render the selection interface into. It will
-        be hidden by the library when not in use. This is a target for the
-        Pinboard library to render the selection interface into. It will be
-        hidden by the library when not in use. This is a target for the Pinboard
-        library to render the selection interface into. It will be hidden by the
-        library when not in use. This is a target for the Pinboard library to
-        render the selection interface into. It will be hidden by the library
-        when not in use.
-      </p>
-    </div>
-    <p>not inside target</p>
-
-    <hr />
     <a href="?expandPinboard=true"
       >Expand pinboard via query param <code>?expandPinboard=true</code></a
     >
@@ -175,6 +149,34 @@ <h4>presented after 1 second</h4>
         and Pinboard detecting that and still taking over the element
       </p>
     </ul>
+    <div style="padding: 20px; background-color: aliceblue">
+      <h3>Pinboard selection target</h3>
+      <div
+        data-pinboard-selection-target
+        data-usage-note="This is a sample usage note, copyright Pinboard sandbox."
+      >
+        <p>
+          This is a target for the <strong>Pinboard</strong> library to render
+          the selection interface into. It will be hidden by the library when
+          not in use. This is a target for the <em>Pinboard</em> library to
+          render the selection interface into. It will be hidden by the library
+          when not in use. This is a target for the
+          <strong>Pinboard</strong> library to render the selection interface
+          into. It will be hidden by the library when not in use.
+        </p>
+        <p>
+          This is a target for the <em>Pinboard</em> library to render the
+          selection interface into. It will be hidden by the library when not in
+          use. This is a target for the <strong>Pinboard</strong> library to
+          render the selection interface into. It will be hidden by the library
+          when not in use. This is a target for the <em>Pinboard</em> library to
+          render the selection interface into. It will be hidden by the library
+          when not in use.
+        </p>
+      </div>
+    </div>
+
+    <hr />
   </body>
   <script>
     setTimeout(

diff --git a/client/src/newswires/newswiresIntegration.tsx b/client/src/newswires/newswiresIntegration.tsx
@@ -16,6 +16,7 @@ const SELECTION_TARGET_DATA_ATTR = "[data-pinboard-selection-target]";
 interface ButtonPosition {
   top: number;
   left: number;
+  unRoundedCorner: "bottom-left" | "top-right" | "top-left" | "bottom-right";
 }
 
 export const NewswiresIntegration = () => {
@@ -49,14 +50,27 @@ export const NewswiresIntegration = () => {
       );
       const firstRect = selectionRects[0];
       const lastRect = selectionRects[selectionRects.length - 1];
-      const newFirstButtonCoords = {
+      const firstButtonCoords = {
         top: firstRect.y - parentRect.y,
         left: firstRect.x - parentRect.x,
       };
-      const newLastButtonCoords = {
+      const firstButtonPosition: ButtonPosition = {
+        ...firstButtonCoords,
+        unRoundedCorner: `bottom-${
+          firstButtonCoords.left > parentRect.width / 2 ? "right" : "left"
+        }`,
+      };
+      const lastButtonCoords = {
         top: lastRect.y - parentRect.y + lastRect.height,
         left: lastRect.x - parentRect.x + lastRect.width - 1,
       };
+      const lastButtonPosition: ButtonPosition = {
+        ...lastButtonCoords,
+        unRoundedCorner: `top-${
+          lastButtonCoords.left > parentRect.width / 2 ? "right" : "left"
+        }`,
+      };
+
       if (maybeClonedTargetEl) {
         console.log(
           "selection contains whole target element; contents:",
@@ -65,8 +79,8 @@ export const NewswiresIntegration = () => {
         setState({
           selectedHTML: maybeClonedTargetEl.innerHTML,
           containerElement: maybeOriginalTargetEl,
-          firstButtonPosition: newFirstButtonCoords,
-          lastButtonPosition: newLastButtonCoords,
+          firstButtonPosition,
+          lastButtonPosition,
         });
       } else if (
         maybeOriginalTargetEl?.contains(selection.anchorNode) &&
@@ -81,10 +95,10 @@ export const NewswiresIntegration = () => {
         setState({
           selectedHTML: tempEl.innerHTML,
           containerElement: maybeOriginalTargetEl,
-          firstButtonPosition: newFirstButtonCoords,
-          lastButtonPosition: newLastButtonCoords,
+          firstButtonPosition,
+          lastButtonPosition,
         });
-        //TODO might need to clean up tempEl
+        //TODO might need to clean up tempEl to avoid memory leak?
       }
     }
   };
@@ -107,8 +121,8 @@ export const NewswiresIntegration = () => {
      *   [x] check parent node of selection is newswires body text el (maybe add data attribute to body text el)
      *       - (find first shared parent of anchorNode and focusNode, make sure we're not sharing bits of text outside of the target)
      *   [x] extract HTML from selection (see chat thread)
-     *   [ ] render button when there's a selection
-     *   [ ] do things with pinboard
+     *   [x] render button when there's a selection
+     *   [x] do things with pinboard
      */
     return () =>
       document.removeEventListener(
@@ -157,10 +171,14 @@ export const NewswiresIntegration = () => {
                     left: ${buttonCoords.left}px;
                     transform: translate(
                       ${
-                        buttonCoords === state.firstButtonPosition
-                          ? "0, -100%"
-                          : "-100%, 0"
-                      }
+                        buttonCoords.unRoundedCorner.includes("left")
+                          ? "0"
+                          : "-100%"
+                      },${
+                    buttonCoords.unRoundedCorner.includes("bottom")
+                      ? "-100%"
+                      : "0"
+                  }
                     );
                     display: flex;
                     align-items: center;
@@ -169,11 +187,7 @@ export const NewswiresIntegration = () => {
                     box-shadow: ${boxShadow};
                     border: none;
                     border-radius: 100px;
-                    border-${
-                      buttonCoords === state.firstButtonPosition
-                        ? "bottom-left"
-                        : "top-right"
-                    }-radius: 0;
+                    border-${buttonCoords.unRoundedCorner}-radius: 0;
                     padding: 0 ${space[2]}px 0 ${space[3]}px;
                     line-height: 2;
                     cursor: pointer;