From ac913a325a0186e05e45cff8eeee452a45efabba Mon Sep 17 00:00:00 2001 From: Jeremy Davis Date: Mon, 4 Nov 2019 16:21:43 +0900 Subject: [PATCH] SONAR-12602 Auto-scroll down when expanding snippets down --- .../issues/components/IssuesSourceViewer.tsx | 5 +- .../ComponentSourceSnippetViewer.tsx | 62 ++++++++-------- .../SnippetViewer.tsx | 30 ++++++-- .../ComponentSourceSnippetViewer-test.tsx | 11 +++ .../__tests__/SnippetViewer-test.tsx | 70 ++++++++++++++++++- .../__tests__/utils-test.ts | 19 +++++ .../__tests__/IssueTitleBar-test.tsx | 10 +++ .../__snapshots__/IssueTitleBar-test.tsx.snap | 6 ++ 8 files changed, 170 insertions(+), 43 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx index 91786a16985..22c337cd48e 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx @@ -58,13 +58,12 @@ export default class IssuesSourceViewer extends React.PureComponent { 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 }); }; 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 dc1736a8af0..1b2d958ca19 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx @@ -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 { + expandBlock = (snippetIndex: number, direction: T.ExpandDirection): Promise => { 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 this.animateBlockExpansion(snippetIndex, direction, newLinesMapped), - () => {} - ); + .then(newLinesMapped => this.animateBlockExpansion(snippetIndex, direction, newLinesMapped)); }; animateBlockExpansion( snippetIndex: number, direction: T.ExpandDirection, newLinesMapped: T.Dict - ) { + ): Promise { if (this.mounted) { const { snippets } = this.state; @@ -218,29 +215,32 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent 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 = () => { 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 4aefbdc7670..621fc9f32f0 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/SnippetViewer.tsx @@ -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; 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; 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 { - node: React.RefObject; + snippetNodeRef: React.RefObject; 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 { } }; + 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 { const displayDuplications = snippet.some(s => !!s.duplicated); return ( -
+
{snippet[0].line > 1 && (
diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx index 26807e530cf..eda23a7515f 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx @@ -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', () => { diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/SnippetViewer-test.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/SnippetViewer-test.tsx index ef8e495d61f..a802a998d9e 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/SnippetViewer-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/SnippetViewer-test.tsx @@ -17,9 +17,10 @@ * 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 = {}) { return shallow( = {}) { /> ); } + +function mountRender(props: Partial = {}) { + return mount( + + ); +} 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 825231cb690..51a995de1bf 100644 --- 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 @@ -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', () => { diff --git a/server/sonar-web/src/main/js/components/issue/components/__tests__/IssueTitleBar-test.tsx b/server/sonar-web/src/main/js/components/issue/components/__tests__/IssueTitleBar-test.tsx index 665a17f8a5e..1699912d706 100644 --- a/server/sonar-web/src/main/js/components/issue/components/__tests__/IssueTitleBar-test.tsx +++ b/server/sonar-web/src/main/js/components/issue/components/__tests__/IssueTitleBar-test.tsx @@ -54,6 +54,16 @@ it('should count all code locations', () => { /> ); expect(element.find('LocationIndex')).toMatchSnapshot(); + + const elementWithLink = shallow( + + ); + expect(elementWithLink.find('LocationIndex')).toMatchSnapshot(); }); it('should have a correct permalink for security hotspots', () => { diff --git a/server/sonar-web/src/main/js/components/issue/components/__tests__/__snapshots__/IssueTitleBar-test.tsx.snap b/server/sonar-web/src/main/js/components/issue/components/__tests__/__snapshots__/IssueTitleBar-test.tsx.snap index 3fc0b16bfe3..ab49356759a 100644 --- a/server/sonar-web/src/main/js/components/issue/components/__tests__/__snapshots__/IssueTitleBar-test.tsx.snap +++ b/server/sonar-web/src/main/js/components/issue/components/__tests__/__snapshots__/IssueTitleBar-test.tsx.snap @@ -6,6 +6,12 @@ exports[`should count all code locations 1`] = ` `; +exports[`should count all code locations 2`] = ` + + 7 + +`; + exports[`should render the titlebar correctly 1`] = `