Combined log point panel icons (#10602)

The log point panel was recently update (#10570) to support multi-line print statements. Part of that change included trying to merge some of the icons (e.g. visible, edit toggle, avatar, and badge picker) so there were fewer possible combinations (which complicated multi-line measuring logic) and so they took up less space overall. Multi-line print statements made it more obvious how our icons were taking up crucial space: <img src="https://github.com/replayio/devtools/assets/29597/cd1b2b53-03fd-475e-b899-89e7e4cecf57)" width="50%" /> Fewer icons would mean more available space which is better for the user when editing large text: <img src="https://github.com/replayio/devtools/assets/29597/83ad2ba9-388e-44ae-a64f-96232bfccbaa)" width="50%" /> This PR combines things a bit further– merging the badge picker and disabled icon, as well as the user avatar and edit icon: https://github.com/replayio/devtools/assets/29597/ce36aec4-373c-48e3-a67a-80b6fb307886 This feels like a good trade off in terms of the concerns I mentioned above.
replayio · Jul 8, 2024 · 925db84 · 925db84
1 parent d5b30c0
commit 925db84
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 18 deletions.
diff --git a/packages/e2e-tests/helpers/source-panel.ts b/packages/e2e-tests/helpers/source-panel.ts
@@ -320,6 +320,7 @@ export async function editLogPoint(
       .locator('[data-test-name="PointPanel-ContentWrapper"] [data-lexical-editor="true"]')
       .isVisible();
     if (!isEditing) {
+      await line.locator('[data-test-name="PointPanel-IconAndAvatar"]').hover();
       await line.locator('[data-test-name="PointPanel-EditButton"]').click();
     }
 

diff --git a/packages/replay-next/components/sources/log-point-panel/LogPointPanel.module.css b/packages/replay-next/components/sources/log-point-panel/LogPointPanel.module.css
@@ -159,7 +159,7 @@
   justify-content: center;
   line-height: 1.5rem;
   height: 1rem;
-  width: 1rem;
+  min-width: 1rem;
   cursor: pointer;
   color: var(--point-panel-input-edit-button-color);
 }
@@ -172,6 +172,11 @@
 .ButtonWithIcon[data-invalid]:hover {
   color: var(--point-panel-input-disabled-cancel-button-color-hover);
 }
+.ButtonWithIcon:disabled,
+.ButtonWithIcon:disabled:hover {
+  cursor: default;
+  color: var(--point-panel-input-edit-button-color);
+}
 
 .ContentInput,
 .ContentInputWithNag {
@@ -211,7 +216,7 @@
   flex: 0 0 1.5rem;
 }
 
-.DisabledIconAndAvatar {
+.IconAndAvatar {
   flex-shrink: 0;
   display: flex;
   align-items: center;
@@ -231,3 +236,13 @@
   word-break: break-word;
   padding: 0.25rem 0;
 }
+
+.ButtonIcon {
+  height: 1rem;
+  width: 1rem;
+}
+
+.DisabledIcon {
+  height: 1rem;
+  width: 1.25rem;
+}
diff --git a/packages/replay-next/components/sources/log-point-panel/LogPointPanel.tsx b/packages/replay-next/components/sources/log-point-panel/LogPointPanel.tsx
@@ -1,5 +1,6 @@
 import { TimeStampedPoint, TimeStampedPointRange } from "@replayio/protocol";
 import {
+  MouseEvent,
   Suspense,
   unstable_useCacheRefresh as useCacheRefresh,
   useContext,
@@ -193,6 +194,7 @@ export function PointPanelWithHitPoints({
   const invalidateCache = useCacheRefresh();
 
   const [isEditing, setIsEditing] = useState(!readOnlyMode && showEditBreakpointNag);
+  const [isHovering, setIsHovering] = useState(false);
   const [editReason, setEditReason] = useState<EditReason | null>(null);
 
   const [isPending, startTransition] = useTransition();
@@ -415,7 +417,7 @@ export function PointPanelWithHitPoints({
                   </div>
 
                   <RemoveConditionalButton
-                    disabled={isPending}
+                    disabled={isPending || !editable}
                     invalid={!isConditionValid}
                     onClick={toggleCondition}
                   />
@@ -432,7 +434,7 @@ export function PointPanelWithHitPoints({
                   </div>
 
                   <RemoveConditionalButton
-                    disabled={isPending}
+                    disabled={isPending || !editable}
                     invalid={!isConditionValid}
                     onClick={toggleCondition}
                   />
@@ -494,12 +496,30 @@ export function PointPanelWithHitPoints({
             data-state-logging-enabled={shouldLog}
             data-test-name="PointPanel-ContentWrapper"
             onClick={() => startEditing("content")}
+            onMouseEnter={() => setIsHovering(true)}
+            onMouseLeave={() => setIsHovering(false)}
           >
-            <BadgePicker
-              disabled={!editable}
-              invalid={!isContentValid}
-              point={pointWithPendingEdits}
-            />
+            {shouldLog ? (
+              <BadgePicker
+                disabled={!editable}
+                invalid={!isContentValid}
+                point={pointWithPendingEdits}
+              />
+            ) : (
+              <button
+                className={styles.ButtonWithIcon}
+                data-test-name="PointPanel-DisabledButton"
+                disabled={isPending}
+                onClick={event => {
+                  event.preventDefault();
+                  event.stopPropagation();
+
+                  toggleShouldLog();
+                }}
+              >
+                <Icon className={styles.DisabledIcon} data-disabled type="toggle-off" />
+              </button>
+            )}
 
             <div className={styles.Content}>
               {isEditing ? (
@@ -532,23 +552,24 @@ export function PointPanelWithHitPoints({
                   fileExtension=".js"
                 />
               )}
-              <div className={styles.DisabledIconAndAvatar}>
+              <div className={styles.IconAndAvatar} data-test-name="PointPanel-IconAndAvatar">
                 {isEditing ? (
                   saveButton
-                ) : editable ? (
+                ) : editable && isHovering ? (
                   <button
                     className={styles.ButtonWithIcon}
                     data-test-name="PointPanel-EditButton"
                     disabled={isPending}
                   >
-                    <Icon className={styles.ButtonIcon} type={shouldLog ? "edit" : "toggle-off"} />
+                    <Icon className={styles.ButtonIcon} type="edit" />
                   </button>
-                ) : null}
-                <AvatarImage
-                  className={styles.CreatedByAvatar}
-                  src={user?.picture || undefined}
-                  title={user?.name || undefined}
-                />
+                ) : (
+                  <AvatarImage
+                    className={styles.CreatedByAvatar}
+                    src={user?.picture || undefined}
+                    title={user?.name || undefined}
+                  />
+                )}
               </div>
             </div>
           </div>