From 9d68778cfae518259e254cf94be1f527fb983da5 Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 01:56:32 -0500
Subject: [PATCH 1/6] improve pen tool

---
 .../src/screenshot/components/PenIcon.tsx     | 44 +++++++++++++++++++
 .../components/ScreenshotEditor.tsx           | 19 ++++----
 .../components/ScreenshotInput.css.ts         | 13 +++++-
 3 files changed, 67 insertions(+), 9 deletions(-)
 create mode 100644 packages/feedback/src/screenshot/components/PenIcon.tsx

diff --git a/packages/feedback/src/screenshot/components/PenIcon.tsx b/packages/feedback/src/screenshot/components/PenIcon.tsx
new file mode 100644
index 000000000000..b0cda1631c07
--- /dev/null
+++ b/packages/feedback/src/screenshot/components/PenIcon.tsx
@@ -0,0 +1,44 @@
+import type { VNode, h as hType } from 'preact';
+
+interface FactoryParams {
+  h: typeof hType;
+}
+
+export default function PenIconFactory({
+  h, // eslint-disable-line @typescript-eslint/no-unused-vars
+}: FactoryParams) {
+  return function PenIcon({ isAnnotating, onClick }: { isAnnotating: boolean; onClick: (e: Event) => void }): VNode {
+    return (
+      <button
+        class="editor__pen-tool"
+        style={{
+          background: isAnnotating
+            ? 'var(--button-primary-background, var(--accent-background))'
+            : 'var(--button-background, var(--background))',
+          color: isAnnotating
+            ? 'var(--button-primary-foreground, var(--accent-foreground))'
+            : 'var(--button-foreground, var(--foreground))',
+        }}
+        onClick={onClick}
+      >
+        <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
+          <path
+            d="M8.5 12L12 8.5L14 11L11 14L8.5 12Z"
+            stroke="currentColor"
+            strokeWidth="1.5"
+            strokeLinecap="round"
+            strokeLinejoin="round"
+          />
+          <path
+            d="M12 8.5L11 3.5L2 2L3.5 11L8.5 12L12 8.5Z"
+            stroke="currentColor"
+            strokeWidth="1.5"
+            strokeLinecap="round"
+            strokeLinejoin="round"
+          />
+          <path d="M2 2L7.5 7.5" stroke="currentColor" strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" />
+        </svg>
+      </button>
+    );
+  };
+}
diff --git a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
index 7cd3aa5f1747..3ac701e009d7 100644
--- a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
+++ b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
@@ -6,6 +6,7 @@ import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-
 import type * as Hooks from 'preact/hooks';
 import { DOCUMENT, WINDOW } from '../../constants';
 import CropCornerFactory from './CropCorner';
+import PenIconFactory from './PenIcon';
 import { createScreenshotInputStyles } from './ScreenshotInput.css';
 import { useTakeScreenshotFactory } from './useTakeScreenshot';
 
@@ -76,6 +77,7 @@ export function ScreenshotEditorFactory({
   return function ScreenshotEditor({ onError }: Props): VNode {
     const styles = hooks.useMemo(() => ({ __html: createScreenshotInputStyles(options.styleNonce).innerText }), []);
     const CropCorner = CropCornerFactory({ h });
+    const PenIcon = PenIconFactory({ h });
 
     const canvasContainerRef = hooks.useRef<HTMLDivElement>(null);
     const cropContainerRef = hooks.useRef<HTMLDivElement>(null);
@@ -393,14 +395,15 @@ export function ScreenshotEditorFactory({
       <div class="editor">
         <style nonce={options.styleNonce} dangerouslySetInnerHTML={styles} />
         {options._experiments.annotations && (
-          <button
-            class="editor__pen-tool"
-            style={{ background: isAnnotating ? 'red' : 'white' }}
-            onClick={e => {
-              e.preventDefault();
-              setIsAnnotating(!isAnnotating);
-            }}
-          ></button>
+          <div class="editor__tool-container">
+            <PenIcon
+              isAnnotating={isAnnotating}
+              onClick={e => {
+                e.preventDefault();
+                setIsAnnotating(!isAnnotating);
+              }}
+            />
+          </div>
         )}
         <div class="editor__canvas-container" ref={canvasContainerRef}>
           <div class="editor__crop-container" style={{ zIndex: isAnnotating ? 1 : 2 }} ref={cropContainerRef}>
diff --git a/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts b/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
index 368a80eaf291..d2ad4111254f 100644
--- a/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
+++ b/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
@@ -15,6 +15,7 @@ export function createScreenshotInputStyles(styleNonce?: string): HTMLStyleEleme
   padding-top: 65px;
   padding-bottom: 65px;
   flex-grow: 1;
+  position: relative;
 
   background-color: ${surface200};
   background-image: repeating-linear-gradient(
@@ -88,9 +89,19 @@ export function createScreenshotInputStyles(styleNonce?: string): HTMLStyleEleme
   border-left: none;
   border-top: none;
 }
+.editor__tool-container {
+  position: absolute;
+  z-index: 2;
+  padding: 10px 0px;
+  top: 0;
+}
 .editor__pen-tool {
-  width: 30px;
   height: 30px;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+  border: var(--button-border, var(--border));
+  border-radius: var(--button-border-radius, 6px);
 }
 `;
 

From 46f74740414ec1e50074885523a3ffa54afa2803 Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 09:49:58 -0500
Subject: [PATCH 2/6] lint again

---
 packages/core/src/types-hoist/feedback/config.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/types-hoist/feedback/config.ts b/packages/core/src/types-hoist/feedback/config.ts
index 4171f2ace941..d7b3d78995bb 100644
--- a/packages/core/src/types-hoist/feedback/config.ts
+++ b/packages/core/src/types-hoist/feedback/config.ts
@@ -64,7 +64,7 @@ export interface FeedbackGeneralConfiguration {
    *
    * Default: undefined
    */
-  _experiments: Partial<{ annotations: boolean; }>
+  _experiments: Partial<{ annotations: boolean }>;
 
   /**
    * Set an object that will be merged sent as tags data with the event.

From c675f44bcf91405804aff95442cb473940c8fc5b Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 09:51:18 -0500
Subject: [PATCH 3/6] oops wrong branch

---
 packages/core/src/types-hoist/feedback/config.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/types-hoist/feedback/config.ts b/packages/core/src/types-hoist/feedback/config.ts
index d7b3d78995bb..4171f2ace941 100644
--- a/packages/core/src/types-hoist/feedback/config.ts
+++ b/packages/core/src/types-hoist/feedback/config.ts
@@ -64,7 +64,7 @@ export interface FeedbackGeneralConfiguration {
    *
    * Default: undefined
    */
-  _experiments: Partial<{ annotations: boolean }>;
+  _experiments: Partial<{ annotations: boolean; }>
 
   /**
    * Set an object that will be merged sent as tags data with the event.

From 1def9e24b5341995428a9645f0a9641fe043ca4b Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 10:09:51 -0500
Subject: [PATCH 4/6] fix css

---
 .../feedback/src/screenshot/components/ScreenshotInput.css.ts  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts b/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
index d2ad4111254f..5b439390d068 100644
--- a/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
+++ b/packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
@@ -56,7 +56,7 @@ export function createScreenshotInputStyles(styleNonce?: string): HTMLStyleEleme
   padding: 8px;
   gap: 8px;
   border-radius: var(--menu-border-radius, 6px);
-  background: var(--button-primary-background, var(--background));
+  background: var(--button-background, var(--background));
   width: 175px;
   position: absolute;
 }
@@ -91,7 +91,6 @@ export function createScreenshotInputStyles(styleNonce?: string): HTMLStyleEleme
 }
 .editor__tool-container {
   position: absolute;
-  z-index: 2;
   padding: 10px 0px;
   top: 0;
 }

From c7e3e01420b6ce36f676d764f022908cc96d860d Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 13:03:06 -0500
Subject: [PATCH 5/6] move button out

---
 .../src/screenshot/components/PenIcon.tsx     | 49 +++++++------------
 .../components/ScreenshotEditor.tsx           | 16 ++++--
 2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/packages/feedback/src/screenshot/components/PenIcon.tsx b/packages/feedback/src/screenshot/components/PenIcon.tsx
index b0cda1631c07..ec50862c1dd4 100644
--- a/packages/feedback/src/screenshot/components/PenIcon.tsx
+++ b/packages/feedback/src/screenshot/components/PenIcon.tsx
@@ -7,38 +7,25 @@ interface FactoryParams {
 export default function PenIconFactory({
   h, // eslint-disable-line @typescript-eslint/no-unused-vars
 }: FactoryParams) {
-  return function PenIcon({ isAnnotating, onClick }: { isAnnotating: boolean; onClick: (e: Event) => void }): VNode {
+  return function PenIcon(): VNode {
     return (
-      <button
-        class="editor__pen-tool"
-        style={{
-          background: isAnnotating
-            ? 'var(--button-primary-background, var(--accent-background))'
-            : 'var(--button-background, var(--background))',
-          color: isAnnotating
-            ? 'var(--button-primary-foreground, var(--accent-foreground))'
-            : 'var(--button-foreground, var(--foreground))',
-        }}
-        onClick={onClick}
-      >
-        <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
-          <path
-            d="M8.5 12L12 8.5L14 11L11 14L8.5 12Z"
-            stroke="currentColor"
-            strokeWidth="1.5"
-            strokeLinecap="round"
-            strokeLinejoin="round"
-          />
-          <path
-            d="M12 8.5L11 3.5L2 2L3.5 11L8.5 12L12 8.5Z"
-            stroke="currentColor"
-            strokeWidth="1.5"
-            strokeLinecap="round"
-            strokeLinejoin="round"
-          />
-          <path d="M2 2L7.5 7.5" stroke="currentColor" strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" />
-        </svg>
-      </button>
+      <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
+        <path
+          d="M8.5 12L12 8.5L14 11L11 14L8.5 12Z"
+          stroke="currentColor"
+          strokeWidth="1.5"
+          strokeLinecap="round"
+          strokeLinejoin="round"
+        />
+        <path
+          d="M12 8.5L11 3.5L2 2L3.5 11L8.5 12L12 8.5Z"
+          stroke="currentColor"
+          strokeWidth="1.5"
+          strokeLinecap="round"
+          strokeLinejoin="round"
+        />
+        <path d="M2 2L7.5 7.5" stroke="currentColor" strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" />
+      </svg>
     );
   };
 }
diff --git a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
index 3ac701e009d7..71898164b69a 100644
--- a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
+++ b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
@@ -396,13 +396,23 @@ export function ScreenshotEditorFactory({
         <style nonce={options.styleNonce} dangerouslySetInnerHTML={styles} />
         {options._experiments.annotations && (
           <div class="editor__tool-container">
-            <PenIcon
-              isAnnotating={isAnnotating}
+            <button
+              class="editor__pen-tool"
+              style={{
+                background: isAnnotating
+                  ? 'var(--button-primary-background, var(--accent-background))'
+                  : 'var(--button-background, var(--background))',
+                color: isAnnotating
+                  ? 'var(--button-primary-foreground, var(--accent-foreground))'
+                  : 'var(--button-foreground, var(--foreground))',
+              }}
               onClick={e => {
                 e.preventDefault();
                 setIsAnnotating(!isAnnotating);
               }}
-            />
+            >
+              <PenIcon></PenIcon>
+            </button>
           </div>
         )}
         <div class="editor__canvas-container" ref={canvasContainerRef}>

From efdfd94e8cb084857506d10bc2efa6d4a94c2b75 Mon Sep 17 00:00:00 2001
From: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date: Tue, 21 Jan 2025 13:37:10 -0500
Subject: [PATCH 6/6] PR comments

---
 .../feedback/src/screenshot/components/ScreenshotEditor.tsx | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
index 71898164b69a..edb626635d63 100644
--- a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
+++ b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
@@ -73,11 +73,11 @@ export function ScreenshotEditorFactory({
   options,
 }: FactoryParams): ComponentType<Props> {
   const useTakeScreenshot = useTakeScreenshotFactory({ hooks });
+  const CropCorner = CropCornerFactory({ h });
+  const PenIcon = PenIconFactory({ h });
 
   return function ScreenshotEditor({ onError }: Props): VNode {
     const styles = hooks.useMemo(() => ({ __html: createScreenshotInputStyles(options.styleNonce).innerText }), []);
-    const CropCorner = CropCornerFactory({ h });
-    const PenIcon = PenIconFactory({ h });
 
     const canvasContainerRef = hooks.useRef<HTMLDivElement>(null);
     const cropContainerRef = hooks.useRef<HTMLDivElement>(null);
@@ -411,7 +411,7 @@ export function ScreenshotEditorFactory({
                 setIsAnnotating(!isAnnotating);
               }}
             >
-              <PenIcon></PenIcon>
+              <PenIcon />
             </button>
           </div>
         )}