Browse Source

fix(perf): Improve full query tooltip layout (#56164)

- Allow specifying query formatter max line length
- Reduce query line length in tooltip
- Bump up the tooltip width

Prevents horizontal scroll bars (yuck) in the tooltip.
George Gritsouk 1 year ago
parent
commit
b81bcedb8d

+ 10 - 6
static/app/views/starfish/components/fullQueryDescription.tsx

@@ -8,11 +8,6 @@ import {SQLishFormatter} from 'sentry/views/starfish/utils/sqlish/SQLishFormatte
 
 const formatter = new SQLishFormatter();
 
-interface Props {
-  group?: string;
-  shortDescription?: string;
-}
-
 export function FullQueryDescription({group, shortDescription}: Props) {
   const {
     data: fullSpan,
@@ -31,10 +26,19 @@ export function FullQueryDescription({group, shortDescription}: Props) {
       <LoadingIndicator mini hideMessage relative />
     </PaddedSpinner>
   ) : (
-    <CodeSnippet language="sql">{formatter.toString(description)}</CodeSnippet>
+    <CodeSnippet language="sql">
+      {formatter.toString(description, {maxLineLength: LINE_LENGTH})}
+    </CodeSnippet>
   );
 }
 
+interface Props {
+  group?: string;
+  shortDescription?: string;
+}
+
+const LINE_LENGTH = 60;
+
 const PaddedSpinner = styled('div')`
   padding: ${space(0)} ${space(0.5)};
 `;

+ 1 - 1
static/app/views/starfish/components/tableCells/spanDescriptionCell.tsx

@@ -75,7 +75,7 @@ const WiderHovercard = styled(
 )`
   &.wider {
     width: auto;
-    max-width: 500px;
+    max-width: 550px;
   }
 `;
 

+ 18 - 0
static/app/views/starfish/utils/sqlish/SQLishFormatter.spec.tsx

@@ -90,6 +90,24 @@ describe('SQLishFormatter', function () {
       `);
     });
 
+    it('Reflows to specified width', () => {
+      expect(
+        formatter.toString(
+          'SELECT "sentry_organization"."id", "sentry_organization"."name", "sentry_organization"."slug", "sentry_organization"."status", "sentry_organization"."date_added" FROM "sentry_organization" WHERE "sentry_organization"."id" = %s LIMIT 21',
+          {maxLineLength: 40}
+        )
+      ).toMatchInlineSnapshot(`
+        "SELECT "sentry_organization"."id",
+          "sentry_organization"."name",
+          "sentry_organization"."slug",
+          "sentry_organization"."status",
+          "sentry_organization"."date_added"
+        FROM "sentry_organization"
+        WHERE "sentry_organization"."id" = %s
+        LIMIT 21"
+      `);
+    });
+
     it('Reflows avoid unnecessary newlines', () => {
       expect(
         formatter.toString(

+ 7 - 5
static/app/views/starfish/utils/sqlish/SQLishFormatter.tsx

@@ -4,6 +4,8 @@ import {simpleMarkup} from 'sentry/views/starfish/utils/sqlish/formatters/simple
 import {string} from 'sentry/views/starfish/utils/sqlish/formatters/string';
 import {SQLishParser} from 'sentry/views/starfish/utils/sqlish/SQLishParser';
 
+type StringFormatterOptions = Parameters<typeof string>[1];
+
 enum Format {
   STRING = 'string',
   SIMPLE_MARKUP = 'simpleMarkup',
@@ -21,17 +23,17 @@ export class SQLishFormatter {
     this.parser = new SQLishParser();
   }
 
-  toString(sql: string) {
-    return this.toFormat(sql, Format.STRING);
+  toString(sql: string, options?: StringFormatterOptions) {
+    return this.toFormat(sql, Format.STRING, options);
   }
 
   toSimpleMarkup(sql: string) {
     return this.toFormat(sql, Format.SIMPLE_MARKUP);
   }
 
-  toFormat(sql: string, format: Format.STRING): string;
+  toFormat(sql: string, format: Format.STRING, options?: StringFormatterOptions): string;
   toFormat(sql: string, format: Format.SIMPLE_MARKUP): React.ReactElement[];
-  toFormat(sql: string, format: Format) {
+  toFormat(sql: string, format: Format, options?: StringFormatterOptions) {
     let tokens;
 
     const sentryTransaction = Sentry.getCurrentHub().getScope()?.getTransaction();
@@ -58,7 +60,7 @@ export class SQLishFormatter {
       return sql;
     }
 
-    const formattedString = FORMATTERS[format](tokens);
+    const formattedString = FORMATTERS[format](tokens, options);
     sentrySpan?.finish();
 
     return formattedString;

+ 6 - 2
static/app/views/starfish/utils/sqlish/formatters/string.ts

@@ -1,7 +1,11 @@
 import {StringAccumulator} from 'sentry/views/starfish/utils/sqlish/formatters/stringAccumulator';
 import type {Token} from 'sentry/views/starfish/utils/sqlish/types';
 
-export function string(tokens: Token[]): string {
+interface Options {
+  maxLineLength?: number;
+}
+
+export function string(tokens: Token[], options: Options = {}): string {
   const accumulator = new StringAccumulator();
 
   let precedingNonWhitespaceToken: Token | undefined = undefined;
@@ -69,7 +73,7 @@ export function string(tokens: Token[]): string {
   }
 
   tokens.forEach(contentize);
-  return accumulator.toString();
+  return accumulator.toString(options.maxLineLength);
 }
 
 // Keywords that always trigger a newline

+ 4 - 4
static/app/views/starfish/utils/sqlish/formatters/stringAccumulator.ts

@@ -40,11 +40,11 @@ export class StringAccumulator {
     this.lastLine.indentTo(level);
   }
 
-  toString() {
+  toString(maxLineLength: number = DEFAULT_MAX_LINE_LENGTH) {
     let output: Line[] = [];
 
     this.lines.forEach(line => {
-      if (line.textLength <= LINE_LENGTH) {
+      if (line.textLength <= maxLineLength) {
         output.push(line);
         return;
       }
@@ -57,7 +57,7 @@ export class StringAccumulator {
 
         const totalLength = (splitLines.at(-1) as Line).textLength + incomingToken.length;
 
-        if (totalLength <= LINE_LENGTH) {
+        if (totalLength <= maxLineLength) {
           splitLines.at(-1)?.add(incomingToken);
         } else {
           splitLines.push(new Line([incomingToken], line.indentation + 1));
@@ -73,7 +73,7 @@ export class StringAccumulator {
   }
 }
 
-const LINE_LENGTH = 100;
+const DEFAULT_MAX_LINE_LENGTH = 100;
 
 class Line {
   tokens: string[];