Commit 87314097 authored by Lee Tickett's avatar Lee Tickett Committed by Illya Klymov

Fix back button when switching mr tabs

parent 68330fb8
......@@ -126,6 +126,13 @@ export default class MergeRequestTabs {
bindEvents() {
$('.merge-request-tabs a[data-toggle="tabvue"]').on('click', this.clickTab);
window.addEventListener('popstate', event => {
if (event.state && event.state.action) {
this.tabShown(event.state.action, event.target.location);
this.currentAction = event.state.action;
this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
}
});
}
// Used in tests
......@@ -155,6 +162,12 @@ export default class MergeRequestTabs {
} else if (action) {
const href = e.currentTarget.getAttribute('href');
this.tabShown(action, href);
if (this.setUrl) {
this.setCurrentAction(action);
}
this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
}
}
}
......@@ -213,11 +226,6 @@ export default class MergeRequestTabs {
this.resetViewContainer();
this.destroyPipelinesView();
}
if (this.setUrl) {
this.setCurrentAction(action);
}
this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
} 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');
......@@ -287,19 +295,25 @@ export default class MergeRequestTabs {
// Ensure parameters and hash come along for the ride
newState += location.search + location.hash;
// TODO: Consider refactoring in light of turbolinks removal.
// Replace the current history state with the new one without breaking
// Turbolinks' history.
//
// See https://github.com/rails/turbolinks/issues/363
window.history.replaceState(
{
url: newState,
},
document.title,
newState,
);
if (window.history.state && window.history.state.url && window.location.pathname !== newState) {
window.history.pushState(
{
url: newState,
action: this.currentAction,
},
document.title,
newState,
);
} else {
window.history.replaceState(
{
url: window.location.href,
action,
},
document.title,
window.location.href,
);
}
return newState;
}
......
---
title: Fix back button when switching MR tabs
merge_request: 29862
author: Lee Tickett
type: fixed
# frozen_string_literal: true
require 'spec_helper'
describe 'User clicks on merge request tabs', :js do
let(:project) { create(:project, :public, :repository) }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
it 'adds entry to page history' do
visit('/')
visit(merge_request_path(merge_request))
click_link('Changes')
expect(current_url).to match(/diffs$/)
page.driver.go_back
expect(current_url).to match(merge_request_path(merge_request))
page.driver.go_back
expect(current_url).to match('/')
end
end
......@@ -30,7 +30,7 @@ describe('MergeRequestTabs', function() {
setLocation();
this.spies = {
history: spyOn(window.history, 'replaceState').and.callFake(function() {}),
history: spyOn(window.history, 'pushState').and.callFake(function() {}),
};
});
......@@ -142,6 +142,7 @@ describe('MergeRequestTabs', function() {
afterEach(() => {
mock.restore();
window.history.replaceState({}, '', '/');
});
it('changes from commits', function() {
......@@ -194,11 +195,21 @@ describe('MergeRequestTabs', function() {
setLocation({
pathname: '/foo/bar/-/merge_requests/1',
});
window.history.replaceState(
{
url: window.location.href,
action: 'show',
},
document.title,
window.location.href,
);
const newState = this.subject('commits');
expect(this.spies.history).toHaveBeenCalledWith(
{
url: newState,
action: 'commits',
},
document.title,
newState,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment