SONAR-12652 Fix 'missing' snippet lines when expanding

diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx
index 15a74e45927aaf1d02febb4f6edc570e4169c704..dc1736a8af02beba85b962d2555d49d36252c223 100644 (file)
--- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx
@@ -87,11 +87,11 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
   }
 
   createSnippetsFromProps() {
-    const snippets = createSnippets(
-      this.props.snippetGroup.locations,
-      this.props.last,
-      this.props.issue.secondaryLocations.length > 0 ? this.props.issue : undefined
-    );
+    const snippets = createSnippets({
+      locations: this.props.snippetGroup.locations,
+      issue: this.props.issue,
+      addIssueLocation: this.props.issue.secondaryLocations.length > 0
+    });
 
     this.setState({ snippets });
   }
@@ -117,6 +117,22 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
     return { wrapper, table };
   }
 
+  /*
+   * Clean after animation
+   */
+  cleanDom(index: number) {
+    const nodes = this.getNodes(index);
+
+    if (!nodes) {
+      return;
+    }
+
+    const { wrapper, table } = nodes;
+
+    table.style.marginTop = '';
+    wrapper.style.maxHeight = '';
+  }
+
   setMaxHeight(index: number, value?: number, up = false) {
     const nodes = this.getNodes(index);
 
@@ -220,6 +236,7 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
           // Wait for transition to finish before updating dom
           setTimeout(() => {
             this.setState({ snippets: newSnippets.filter(s => !s.toDelete) });
+            this.cleanDom(snippetIndex);
           }, 200);
         }
       );

diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx
index a932293f62b2b0ecb15ae295b7010820d14689eb..4aefbdc7670978eda9fede310f249c1a4ffea979 100644 (file)
--- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx
@@ -17,8 +17,8 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-import * as React from 'react';
 import classNames from 'classnames';
+import * as React from 'react';
 import ExpandSnippetIcon from 'sonar-ui-common/components/icons/ExpandSnippetIcon';
 import { translate } from 'sonar-ui-common/helpers/l10n';
 import { scrollHorizontally } from 'sonar-ui-common/helpers/scrolling';
@@ -30,7 +30,7 @@ import {
   optimizeLocationMessage,
   optimizeSelectedIssue
 } from '../../../components/SourceViewer/helpers/lines';
-import { inSnippet, LINES_BELOW_LAST } from './utils';
+import { inSnippet, LINES_BELOW_ISSUE } from './utils';
 
 interface Props {
   branchLike: T.BranchLike | undefined;
@@ -185,7 +185,7 @@ export default class SnippetViewer extends React.PureComponent<Props> {
         .filter(l => inSnippet(l, snippet) && (l === issueLine || openIssuesByLine[l]))
     );
     const verticalBuffer = last
-      ? Math.max(0, LINES_BELOW_LAST - (bottomLine - lowestVisibleIssue))
+      ? Math.max(0, LINES_BELOW_ISSUE - (bottomLine - lowestVisibleIssue))
       : 0;
 
     const displayDuplications = snippet.some(s => !!s.duplicated);

diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts
index f33a99bccbf16c04f7fe48ac4d08f69213144785..1492af9650162ef5d379ffc496e459374c23b8ee 100644 (file)
--- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts
+++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts
@@ -82,97 +82,108 @@ describe('groupLocationsByComponent', () => {
 
 describe('createSnippets', () => {
   it('should merge snippets correctly', () => {
-    const results = createSnippets(
-      [
-        mockFlowLocation({
-          textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 19, startOffset: 2, endLine: 19, endOffset: 3 }
-        })
-      ],
-      false
-    );
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 19, startOffset: 2, endLine: 19, endOffset: 3 }
+      })
+    ];
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, locations[1]),
+      addIssueLocation: false
+    });
 
     expect(results).toHaveLength(1);
-    expect(results[0]).toEqual({ index: 0, start: 14, end: 21 });
+    expect(results[0]).toEqual({ index: 0, start: 14, end: 28 });
   });
 
   it('should merge snippets correctly, even when not in sequence', () => {
-    const results = createSnippets(
-      [
-        mockFlowLocation({
-          textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 14, startOffset: 2, endLine: 14, endOffset: 3 }
-        })
-      ],
-      false
-    );
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 14, startOffset: 2, endLine: 14, endOffset: 3 }
+      })
+    ];
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, locations[2]),
+      addIssueLocation: false
+    });
 
     expect(results).toHaveLength(2);
-    expect(results[0]).toEqual({ index: 0, start: 12, end: 18 });
+    expect(results[0]).toEqual({ index: 0, start: 12, end: 23 });
     expect(results[1]).toEqual({ index: 1, start: 45, end: 49 });
   });
 
   it('should merge three snippets together', () => {
-    const results = createSnippets(
-      [
-        mockFlowLocation({
-          textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 22, startOffset: 2, endLine: 22, endOffset: 3 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 18, startOffset: 2, endLine: 18, endOffset: 3 }
-        })
-      ],
-      false
-    );
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 23, startOffset: 2, endLine: 23, endOffset: 3 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 18, startOffset: 2, endLine: 18, endOffset: 3 }
+      })
+    ];
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, locations[0]),
+      addIssueLocation: false
+    });
 
     expect(results).toHaveLength(2);
-    expect(results[0]).toEqual({ index: 0, start: 14, end: 24 });
+    expect(results[0]).toEqual({ index: 0, start: 14, end: 25 });
     expect(results[1]).toEqual({ index: 1, start: 45, end: 49 });
   });
 
   it("should prepend the issue's main location if necessary", () => {
-    const results = createSnippets(
-      [
-        mockFlowLocation({
-          textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 22, startOffset: 2, endLine: 22, endOffset: 3 }
-        })
-      ],
-      false,
-      mockIssue(false, { textRange: { startLine: 5, endLine: 5, startOffset: 0, endOffset: 0 } })
-    );
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 22, startOffset: 2, endLine: 22, endOffset: 3 }
+      })
+    ];
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, {
+        textRange: { startLine: 5, endLine: 5, startOffset: 0, endOffset: 0 }
+      }),
+      addIssueLocation: true
+    });
 
     expect(results).toHaveLength(3);
     expect(results[0]).toEqual({ index: 0, start: 3, end: 14 });
   });
 
   it('should handle last component', () => {
-    const results = createSnippets(
-      [
-        mockFlowLocation({
-          textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
-        }),
-        mockFlowLocation({
-          textRange: { startLine: 19, startOffset: 2, endLine: 19, endOffset: 3 }
-        })
-      ],
-      true
-    );
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
+      }),
+      mockFlowLocation({
+        textRange: { startLine: 19, startOffset: 2, endLine: 19, endOffset: 3 }
+      })
+    ];
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, locations[1]),
+      addIssueLocation: false
+    });
 
     expect(results).toHaveLength(1);
     expect(results[0]).toEqual({ index: 0, start: 14, end: 28 });

diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts
index b8938650d5fd6b26d0c1f7363ac098ba5920ded9..9a5e7907b910a81553883cdb5c642523e9cc9af0 100644 (file)
--- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts
+++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts
@@ -20,7 +20,7 @@
 const LINES_ABOVE = 2;
 const LINES_BELOW = 2;
 export const MERGE_DISTANCE = 4; // Merge if snippets are four lines away (separated by 3 lines) or fewer
-export const LINES_BELOW_LAST = 9;
+export const LINES_BELOW_ISSUE = 9;
 export const EXPAND_BY_LINES = 10;
 
 function unknownComponent(key: string): T.SnippetsByComponent {
@@ -54,21 +54,29 @@ function getPrimaryLocation(issue: T.Issue): T.FlowLocation {
   };
 }
 
-export function createSnippets(
-  locations: T.FlowLocation[],
-  last: boolean,
-  issue?: T.Issue
-): T.Snippet[] {
+function addLinesBellow(params: { issue: T.Issue; locationEnd: number }) {
+  const { issue, locationEnd } = params;
+  const issueEndLine = (issue.textRange && issue.textRange.endLine) || 0;
+
+  if (!issueEndLine || issueEndLine === locationEnd) {
+    return locationEnd + LINES_BELOW_ISSUE;
+  }
+
+  return locationEnd + LINES_BELOW;
+}
+
+export function createSnippets(params: {
+  locations: T.FlowLocation[];
+  issue: T.Issue;
+  addIssueLocation: boolean;
+}): T.Snippet[] {
+  const { locations, issue, addIssueLocation } = params;
   // For each location's range (2 above and 2 below), and then compare with other ranges
   // to merge snippets that collide.
-  return (issue ? [getPrimaryLocation(issue), ...locations] : locations).reduce(
+  return (addIssueLocation ? [getPrimaryLocation(issue), ...locations] : locations).reduce(
     (snippets: T.Snippet[], loc, index) => {
       const startIndex = Math.max(1, loc.textRange.startLine - LINES_ABOVE);
-      const endIndex =
-        loc.textRange.endLine +
-        ((issue && index === 0) || (last && index === locations.length - 1)
-          ? LINES_BELOW_LAST
-          : LINES_BELOW);
+      const endIndex = addLinesBellow({ issue, locationEnd: loc.textRange.endLine });
 
       let firstCollision: { start: number; end: number } | undefined;