diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 21fe4356fe0350925d945014077cbe1bc1dd9ddc..540959b3a4643cf1f38e75b911517331dfbf4edf 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -64,6 +64,8 @@ import syntaxHighlight from './syntax_highlight'; // // +// <100ms is typically indistinguishable from "instant" for users, but allows for re-rendering +const FAST_DELAY_FOR_RERENDER = 75; // Store the `location` object, allowing for easier stubbing in tests let { location } = window; @@ -83,6 +85,8 @@ export default class MergeRequestTabs { this.peek = document.getElementById('js-peek'); this.paddingTop = 16; + this.scrollPositions = {}; + this.commitsTab = document.querySelector('.tab-content .commits.tab-pane'); this.currentTab = null; @@ -136,11 +140,30 @@ export default class MergeRequestTabs { } } + storeScroll() { + if (this.currentTab) { + this.scrollPositions[this.currentTab] = document.documentElement.scrollTop; + } + } + recallScroll(action) { + const storedPosition = this.scrollPositions[action]; + + setTimeout(() => { + window.scrollTo({ + top: storedPosition && storedPosition > 0 ? storedPosition : 0, + left: 0, + behavior: 'auto', + }); + }, FAST_DELAY_FOR_RERENDER); + } + clickTab(e) { if (e.currentTarget) { e.stopImmediatePropagation(); e.preventDefault(); + this.storeScroll(); + const { action } = e.currentTarget.dataset || {}; if (isMetaClick(e)) { @@ -210,8 +233,14 @@ export default class MergeRequestTabs { this.resetViewContainer(); this.mountPipelinesView(); } else { - this.mergeRequestTabPanes.querySelector('#notes').style.display = 'block'; - this.mergeRequestTabs.querySelector('.notes-tab').classList.add('active'); + const notesTab = this.mergeRequestTabs.querySelector('.notes-tab'); + const notesPane = this.mergeRequestTabPanes.querySelector('#notes'); + if (notesPane) { + notesPane.style.display = 'block'; + } + if (notesTab) { + notesTab.classList.add('active'); + } if (bp.getBreakpointSize() !== 'xs') { this.expandView(); @@ -221,6 +250,8 @@ export default class MergeRequestTabs { } $('.detail-page-description').renderGFM(); + + this.recallScroll(action); } else if (action === this.currentAction) { // ContentTop is used to handle anything at the top of the page before the main content const mainContentContainer = document.querySelector('.content-wrapper'); diff --git a/spec/frontend/merge_request_tabs_spec.js b/spec/frontend/merge_request_tabs_spec.js index 23e9bf8b44792f85752681713d0a959ad34d995d..ced9b71125b78369ce7db41cdb6207dc3b5f3a90 100644 --- a/spec/frontend/merge_request_tabs_spec.js +++ b/spec/frontend/merge_request_tabs_spec.js @@ -34,6 +34,44 @@ describe('MergeRequestTabs', () => { gl.mrWidget = {}; }); + describe('clickTab', () => { + let params; + + beforeEach(() => { + document.documentElement.scrollTop = 100; + + params = { + metaKey: false, + ctrlKey: false, + which: 1, + stopImmediatePropagation() {}, + preventDefault() {}, + currentTarget: { + getAttribute(attr) { + return attr === 'href' ? 'a/tab/url' : null; + }, + }, + }; + }); + + it("stores the current scroll position if there's an active tab", () => { + testContext.class.currentTab = 'someTab'; + + testContext.class.clickTab(params); + + expect(testContext.class.scrollPositions.someTab).toBe(100); + }); + + it("doesn't store a scroll position if there's no active tab", () => { + // this happens on first load, and we just don't want to store empty values in the `null` property + testContext.class.currentTab = null; + + testContext.class.clickTab(params); + + expect(testContext.class.scrollPositions).toEqual({}); + }); + }); + describe('opensInNewTab', () => { const windowTarget = '_blank'; let clickTabParams; @@ -258,6 +296,7 @@ describe('MergeRequestTabs', () => { beforeEach(() => { jest.spyOn(mainContent, 'getBoundingClientRect').mockReturnValue({ top: 10 }); jest.spyOn(tabContent, 'getBoundingClientRect').mockReturnValue({ top: 100 }); + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); jest.spyOn(document, 'querySelector').mockImplementation((selector) => { return selector === '.content-wrapper' ? mainContent : tabContent; }); @@ -267,8 +306,6 @@ describe('MergeRequestTabs', () => { it('calls window scrollTo with options if document has scrollBehavior', () => { document.documentElement.style.scrollBehavior = ''; - jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); - testContext.class.tabShown('commits', 'foobar'); expect(window.scrollTo.mock.calls[0][0]).toEqual({ top: 39, behavior: 'smooth' }); @@ -276,11 +313,50 @@ describe('MergeRequestTabs', () => { it('calls window scrollTo with two args if document does not have scrollBehavior', () => { jest.spyOn(document.documentElement, 'style', 'get').mockReturnValue({}); - jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); testContext.class.tabShown('commits', 'foobar'); expect(window.scrollTo.mock.calls[0]).toEqual([0, 39]); }); + + describe('when switching tabs', () => { + const SCROLL_TOP = 100; + + beforeAll(() => { + jest.useFakeTimers(); + }); + + beforeEach(() => { + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); + testContext.class.mergeRequestTabs = document.createElement('div'); + testContext.class.mergeRequestTabPanes = document.createElement('div'); + testContext.class.currentTab = 'tab'; + testContext.class.scrollPositions = { newTab: SCROLL_TOP }; + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + it('scrolls to the stored position, if one is stored', () => { + testContext.class.tabShown('newTab'); + + jest.advanceTimersByTime(250); + + expect(window.scrollTo.mock.calls[0][0]).toEqual({ + top: SCROLL_TOP, + left: 0, + behavior: 'auto', + }); + }); + + it('scrolls to 0, if no position is stored', () => { + testContext.class.tabShown('unknownTab'); + + jest.advanceTimersByTime(250); + + expect(window.scrollTo.mock.calls[0][0]).toEqual({ top: 0, left: 0, behavior: 'auto' }); + }); + }); }); });