]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-12602 Auto-scroll down when expanding snippets down
authorJeremy Davis <jeremy.davis@sonarsource.com>
Mon, 4 Nov 2019 07:21:43 +0000 (16:21 +0900)
committerSonarTech <sonartech@sonarsource.com>
Thu, 7 Nov 2019 10:45:17 +0000 (11:45 +0100)
server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/SnippetViewer-test.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts
server/sonar-web/src/main/js/components/issue/components/__tests__/IssueTitleBar-test.tsx
server/sonar-web/src/main/js/components/issue/components/__tests__/__snapshots__/IssueTitleBar-test.tsx.snap

index 91786a16985fdf8b8d2e87499a9db422f253bcbd..22c337cd48e4fc5618b2625e4a9f14277e8f9492 100644 (file)
@@ -58,13 +58,12 @@ export default class IssuesSourceViewer extends React.PureComponent<Props> {
     if (this.node) {
       const element = this.node.querySelector(`[data-issue="${this.props.openIssue.key}"]`);
       if (element) {
-        this.handleScroll(element, smooth);
+        this.handleScroll(element, undefined, smooth);
       }
     }
   };
 
-  handleScroll = (element: Element, smooth = true) => {
-    const offset = window.innerHeight / 2;
+  handleScroll = (element: Element, offset = window.innerHeight / 2, smooth = true) => {
     scrollToElement(element, { topOffset: offset - 100, bottomOffset: offset, smooth });
   };
 
index dc1736a8af02beba85b962d2555d49d36252c223..1b2d958ca1950d2084073d433f8f0f81cd7192b4 100644 (file)
@@ -54,7 +54,7 @@ interface Props {
     index: number,
     line: number
   ) => React.ReactNode;
-  scroll?: (element: HTMLElement) => void;
+  scroll?: (element: HTMLElement, offset: number) => void;
   snippetGroup: T.SnippetGroup;
 }
 
@@ -160,13 +160,13 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
     }
   }
 
-  expandBlock = (snippetIndex: number, direction: T.ExpandDirection) => {
+  expandBlock = (snippetIndex: number, direction: T.ExpandDirection): Promise<void> => {
     const { branchLike, snippetGroup } = this.props;
     const { key } = snippetGroup.component;
     const { snippets } = this.state;
     const snippet = snippets.find(s => s.index === snippetIndex);
     if (!snippet) {
-      return;
+      return Promise.reject();
     }
     // Extend by EXPAND_BY_LINES and add buffer for merging snippets
     const extension = EXPAND_BY_LINES + MERGE_DISTANCE - 1;
@@ -180,7 +180,7 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
             from: snippet.end + 1,
             to: snippet.end + extension
           };
-    getSources({
+    return getSources({
       key,
       ...range,
       ...getBranchLikeQuery(branchLike)
@@ -192,17 +192,14 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
           return lineMap;
         }, {})
       )
-      .then(
-        newLinesMapped => this.animateBlockExpansion(snippetIndex, direction, newLinesMapped),
-        () => {}
-      );
+      .then(newLinesMapped => this.animateBlockExpansion(snippetIndex, direction, newLinesMapped));
   };
 
   animateBlockExpansion(
     snippetIndex: number,
     direction: T.ExpandDirection,
     newLinesMapped: T.Dict<T.SourceLine>
-  ) {
+  ): Promise<void> {
     if (this.mounted) {
       const { snippets } = this.state;
 
@@ -218,29 +215,32 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
       deletedSnippets.forEach(s => this.setMaxHeight(s.index));
       this.setMaxHeight(snippetIndex);
 
-      this.setState(
-        ({ additionalLines, snippets }) => {
-          const combinedLines = { ...additionalLines, ...newLinesMapped };
-          return {
-            additionalLines: combinedLines,
-            snippets
-          };
-        },
-        () => {
-          // Set max-height 0 to trigger CSS transitions
-          deletedSnippets.forEach(s => {
-            this.setMaxHeight(s.index, 0);
-          });
-          this.setMaxHeight(snippetIndex, undefined, direction === 'up');
-
-          // Wait for transition to finish before updating dom
-          setTimeout(() => {
-            this.setState({ snippets: newSnippets.filter(s => !s.toDelete) });
-            this.cleanDom(snippetIndex);
-          }, 200);
-        }
-      );
+      return new Promise(resolve => {
+        this.setState(
+          ({ additionalLines, snippets }) => {
+            const combinedLines = { ...additionalLines, ...newLinesMapped };
+            return {
+              additionalLines: combinedLines,
+              snippets
+            };
+          },
+          () => {
+            // Set max-height 0 to trigger CSS transitions
+            deletedSnippets.forEach(s => {
+              this.setMaxHeight(s.index, 0);
+            });
+            this.setMaxHeight(snippetIndex, undefined, direction === 'up');
+
+            // Wait for transition to finish before updating dom
+            setTimeout(() => {
+              this.setState({ snippets: newSnippets.filter(s => !s.toDelete) }, resolve);
+              this.cleanDom(snippetIndex);
+            }, 200);
+          }
+        );
+      });
     }
+    return Promise.resolve();
   }
 
   expandComponent = () => {
index 4aefbdc7670978eda9fede310f249c1a4ffea979..621fc9f32f07aae22746438b5df6e5b667876a55 100644 (file)
@@ -37,7 +37,7 @@ interface Props {
   component: T.SourceViewerFile;
   duplications?: T.Duplication[];
   duplicationsByLine?: { [line: number]: number[] };
-  expandBlock: (snippetIndex: number, direction: T.ExpandDirection) => void;
+  expandBlock: (snippetIndex: number, direction: T.ExpandDirection) => Promise<void>;
   handleCloseIssues: (line: T.SourceLine) => void;
   handleLinePopupToggle: (line: T.SourceLine) => void;
   handleOpenIssues: (line: T.SourceLine) => void;
@@ -58,25 +58,25 @@ interface Props {
   onLocationSelect: (index: number) => void;
   openIssuesByLine: T.Dict<boolean>;
   renderDuplicationPopup: (index: number, line: number) => React.ReactNode;
-  scroll?: (element: HTMLElement) => void;
+  scroll?: (element: HTMLElement, offset?: number) => void;
   snippet: T.SourceLine[];
 }
 
 const SCROLL_LEFT_OFFSET = 32;
 
 export default class SnippetViewer extends React.PureComponent<Props> {
-  node: React.RefObject<HTMLDivElement>;
+  snippetNodeRef: React.RefObject<HTMLDivElement>;
 
   constructor(props: Props) {
     super(props);
-    this.node = React.createRef();
+    this.snippetNodeRef = React.createRef();
   }
 
   doScroll = (element: HTMLElement) => {
     if (this.props.scroll) {
       this.props.scroll(element);
     }
-    const parent = this.node.current as Element;
+    const parent = this.snippetNodeRef.current as Element;
 
     if (parent) {
       scrollHorizontally(element, {
@@ -87,8 +87,24 @@ export default class SnippetViewer extends React.PureComponent<Props> {
     }
   };
 
+  scrollToLastExpandedRow = () => {
+    if (this.props.scroll) {
+      const snippetNode = this.snippetNodeRef.current as Element;
+      if (!snippetNode) {
+        return;
+      }
+      const rows = snippetNode.querySelectorAll('tr');
+      const lastRow = rows[rows.length - 1];
+      this.props.scroll(lastRow, 100);
+    }
+  };
+
   expandBlock = (direction: T.ExpandDirection) => () =>
-    this.props.expandBlock(this.props.index, direction);
+    this.props.expandBlock(this.props.index, direction).then(() => {
+      if (direction === 'down') {
+        this.scrollToLastExpandedRow();
+      }
+    });
 
   renderLine({
     displayDuplications,
@@ -191,7 +207,7 @@ export default class SnippetViewer extends React.PureComponent<Props> {
     const displayDuplications = snippet.some(s => !!s.duplicated);
 
     return (
-      <div className="source-viewer-code snippet" ref={this.node}>
+      <div className="source-viewer-code snippet" ref={this.snippetNodeRef}>
         <div>
           {snippet[0].line > 1 && (
             <div className="expand-block expand-block-above">
index 26807e530cf383f7f58f628282abb9c4dab9625c..eda23a7515fc01b5ba3023a266920df90b32edbc 100644 (file)
@@ -263,6 +263,17 @@ describe('getNodes', () => {
     mockDom(rootNode.current!);
     expect(wrapper.instance().getNodes(3)).not.toBeUndefined();
   });
+
+  it('should enable cleaning the dom', async () => {
+    await waitAndUpdate(wrapper);
+    const rootNode = wrapper.instance().rootNodeRef;
+    mockDom(rootNode.current!);
+
+    expect(wrapper.instance().cleanDom(3));
+    const nodes = wrapper.instance().getNodes(3);
+    expect(nodes!.wrapper.style.maxHeight).toBe('');
+    expect(nodes!.table.style.marginTop).toBe('');
+  });
 });
 
 describe('getHeight', () => {
index ef8e495d61fa9c66b2f1740a94ec48ed2dbd2adc..a802a998d9e5e086417289c494c9b1a164a277b2 100644 (file)
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-import { shallow } from 'enzyme';
+import { mount, shallow } from 'enzyme';
 import { range } from 'lodash';
 import * as React from 'react';
+import { scrollHorizontally } from 'sonar-ui-common/helpers/scrolling';
 import {
   mockIssue,
   mockMainBranch,
@@ -28,6 +29,14 @@ import {
 } from '../../../../helpers/testMocks';
 import SnippetViewer from '../SnippetViewer';
 
+jest.mock('sonar-ui-common/helpers/scrolling', () => ({
+  scrollHorizontally: jest.fn()
+}));
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});
+
 it('should render correctly', () => {
   const snippet = range(5, 8).map(line => mockSourceLine({ line }));
   const wrapper = shallowRender({
@@ -59,7 +68,7 @@ it('should render correctly when at the bottom of the file', () => {
 
 it('should correctly handle expansion', () => {
   const snippet = range(5, 8).map(line => mockSourceLine({ line }));
-  const expandBlock = jest.fn();
+  const expandBlock = jest.fn(() => Promise.resolve());
 
   const wrapper = shallowRender({
     expandBlock,
@@ -80,6 +89,29 @@ it('should correctly handle expansion', () => {
   expect(expandBlock).toHaveBeenCalledWith(2, 'down');
 });
 
+it('should handle scrolling', () => {
+  const scroll = jest.fn();
+  const wrapper = mountRender({ scroll });
+
+  const element = {} as HTMLElement;
+
+  wrapper.instance().doScroll(element);
+
+  expect(scroll).toHaveBeenCalledWith(element);
+
+  expect(scrollHorizontally).toHaveBeenCalled();
+  expect((scrollHorizontally as jest.Mock).mock.calls[0][0]).toBe(element);
+});
+
+it('should handle scrolling to expanded row', () => {
+  const scroll = jest.fn();
+  const wrapper = mountRender({ scroll });
+
+  wrapper.instance().scrollToLastExpandedRow();
+
+  expect(scroll).toHaveBeenCalled();
+});
+
 function shallowRender(props: Partial<SnippetViewer['props']> = {}) {
   return shallow<SnippetViewer>(
     <SnippetViewer
@@ -113,3 +145,37 @@ function shallowRender(props: Partial<SnippetViewer['props']> = {}) {
     />
   );
 }
+
+function mountRender(props: Partial<SnippetViewer['props']> = {}) {
+  return mount<SnippetViewer>(
+    <SnippetViewer
+      branchLike={mockMainBranch()}
+      component={mockSourceViewerFile()}
+      duplications={undefined}
+      duplicationsByLine={undefined}
+      expandBlock={jest.fn()}
+      handleCloseIssues={jest.fn()}
+      handleLinePopupToggle={jest.fn()}
+      handleOpenIssues={jest.fn()}
+      handleSymbolClick={jest.fn()}
+      highlightedLocationMessage={{ index: 0, text: '' }}
+      highlightedSymbols={[]}
+      index={0}
+      issue={mockIssue()}
+      issuesByLine={{}}
+      last={false}
+      linePopup={undefined}
+      loadDuplications={jest.fn()}
+      locations={[]}
+      locationsByLine={{}}
+      onIssueChange={jest.fn()}
+      onIssuePopupToggle={jest.fn()}
+      onLocationSelect={jest.fn()}
+      openIssuesByLine={{}}
+      renderDuplicationPopup={jest.fn()}
+      scroll={jest.fn()}
+      snippet={[mockSourceLine()]}
+      {...props}
+    />
+  );
+}
index 825231cb69069166ec32174633bef5c60044cc79..51a995de1bf6c071c174b4a446733bc5fec4bda9 100644 (file)
@@ -169,6 +169,25 @@ describe('createSnippets', () => {
     expect(results).toHaveLength(3);
     expect(results[0]).toEqual({ index: 0, start: 7, end: 21 });
   });
+
+  it('should work for location with no textrange', () => {
+    const locations = [
+      mockFlowLocation({
+        textRange: { startLine: 85, startOffset: 2, endLine: 85, endOffset: 3 }
+      })
+    ];
+
+    const results = createSnippets({
+      locations,
+      issue: mockIssue(false, {
+        textRange: undefined
+      }),
+      addIssueLocation: true
+    });
+
+    expect(results).toHaveLength(2);
+    expect(results[0]).toEqual({ index: 0, start: 1, end: 9 });
+  });
 });
 
 describe('expandSnippet', () => {
index 665a17f8a5e13346ae1a88058b5e54cdbe30802e..1699912d706b0ae662c0b66b86a61b5e824d085e 100644 (file)
@@ -54,6 +54,16 @@ it('should count all code locations', () => {
     />
   );
   expect(element.find('LocationIndex')).toMatchSnapshot();
+
+  const elementWithLink = shallow(
+    <IssueTitleBar
+      displayLocationsCount={true}
+      displayLocationsLink={true}
+      issue={issueWithLocations}
+      togglePopup={jest.fn()}
+    />
+  );
+  expect(elementWithLink.find('LocationIndex')).toMatchSnapshot();
 });
 
 it('should have a correct permalink for security hotspots', () => {
index 3fc0b16bfe37e4e63287b741e5ee4745e83d7258..ab49356759aa1c1227244d5a82633ec42b3dfd0e 100644 (file)
@@ -6,6 +6,12 @@ exports[`should count all code locations 1`] = `
 </LocationIndex>
 `;
 
+exports[`should count all code locations 2`] = `
+<LocationIndex>
+  7
+</LocationIndex>
+`;
+
 exports[`should render the titlebar correctly 1`] = `
 <div
   className="issue-row"