From e6a6b3e31a3beb40a4f8866a5c84bc8c24be63e8 Mon Sep 17 00:00:00 2001
From: Matias Chomicki <matyax@gmail.com>
Date: Tue, 10 Dec 2024 15:18:50 +0000
Subject: [PATCH] Logs Panel: Allow text selection without changing Log Details
 state (#96995)

* LogRows: allow text selection even without popover menu

* Add unit test
---
 .../features/logs/components/LogRows.test.tsx | 22 +++++++++++++++++++
 .../app/features/logs/components/LogRows.tsx  | 12 +++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/public/app/features/logs/components/LogRows.test.tsx b/public/app/features/logs/components/LogRows.test.tsx
index 439bf7087fc..cd861e2e37d 100644
--- a/public/app/features/logs/components/LogRows.test.tsx
+++ b/public/app/features/logs/components/LogRows.test.tsx
@@ -275,4 +275,26 @@ describe('Popover menu', () => {
     expect(onClickFilterOutString).toHaveBeenCalledTimes(1);
     expect(onClickFilterString).toHaveBeenCalledTimes(1);
   });
+  describe('Interacting with log details', () => {
+    it('Allows text selection even if the popover menu is not available', async () => {
+      setup({
+        onClickFilterOutString: undefined,
+        onClickFilterString: undefined,
+      });
+      await userEvent.click(screen.getByText('log message 1'));
+      expect(screen.queryByText('Copy selection')).not.toBeInTheDocument();
+      expect(screen.queryByText(/details/)).not.toBeInTheDocument();
+    });
+
+    it('Displays Log Details if there is no text selection', async () => {
+      jest.spyOn(document, 'getSelection').mockReturnValue(null);
+      setup({
+        onClickFilterOutString: undefined,
+        onClickFilterString: undefined,
+      });
+      await userEvent.click(screen.getByText('log message 1'));
+      expect(screen.queryByText('Copy selection')).not.toBeInTheDocument();
+      expect(screen.getByText(/details/)).toBeInTheDocument();
+    });
+  });
 });
diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx
index 96d1bd2e71e..94b35c7d81d 100644
--- a/public/app/features/logs/components/LogRows.tsx
+++ b/public/app/features/logs/components/LogRows.tsx
@@ -115,13 +115,15 @@ class UnThemedLogRows extends PureComponent<Props, State> {
   }
 
   handleSelection = (e: MouseEvent<HTMLTableRowElement>, row: LogRowModel): boolean => {
-    if (this.popoverMenuSupported() === false) {
-      return false;
-    }
     const selection = document.getSelection()?.toString();
     if (!selection) {
       return false;
     }
+    if (this.popoverMenuSupported() === false) {
+      // This signals onRowClick inside LogRow to skip the event because the user is selecting text
+      return selection ? true : false;
+    }
+
     if (!this.logRowsRef.current) {
       return false;
     }
@@ -247,7 +249,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
                   pinLineButtonTooltipTitle={this.props.pinLineButtonTooltipTitle}
                   pinned={this.props.pinnedRowId === row.uid || pinnedLogs?.some((logId) => logId === row.rowId)}
                   isFilterLabelActive={this.props.isFilterLabelActive}
-                  handleTextSelection={this.popoverMenuSupported() ? this.handleSelection : undefined}
+                  handleTextSelection={this.handleSelection}
                   {...rest}
                 />
               ))}
@@ -270,7 +272,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
                   pinLineButtonTooltipTitle={this.props.pinLineButtonTooltipTitle}
                   pinned={this.props.pinnedRowId === row.uid || pinnedLogs?.some((logId) => logId === row.rowId)}
                   isFilterLabelActive={this.props.isFilterLabelActive}
-                  handleTextSelection={this.popoverMenuSupported() ? this.handleSelection : undefined}
+                  handleTextSelection={this.handleSelection}
                   {...rest}
                 />
               ))}