Skip to content

Change diff view type asynchronously on Create new MR page

Thomas Randolph requested to merge tor/maintenance/no-refresh-change-mr-create-diff-view into master

What does this MR do and why?

For #358217 (closed)

Problem

When switching view types (between inline and side-by-side) on the Create New MR page, the entire page is reloaded, which loses a potentially substantial amount of entered-but-unsaved data.
Note: The browser always interrupts the reload to prompt about unsaved data; the user must approve the navigation for the data loss to occur.

Solution

This MR causes a Diff reload to occur regardless of page load status.

Normally, once the Changes tab has been loaded, that's the only time the Diffs endpoint will load the files.
The built-in assumption is that any changes (like which view type) will be present on the page due to a full reload, so we'd never need to switch the endpoint or request it a second time.

Because a full page reload is pretty offensive from a UX perspective, the changes here build in an alternate path where the endpoint changes based on a button click, and the asynchronous load can happen more than once.

Screenshots or screen recordings

It's quite difficult to track what's happening in the before video (that's the whole problem this MR is solving!).

  1. The first click changes from the Commits to the Changes tab. This jumps to the top of the page.
  2. The next click - to switch view types - prompts for leaving the page because there's unsaved content.
    • The unsaved content (e.g. description or any assigned labels, etc.) is gone
  3. The next click - again to switch view type - doesn't prompt (because there's no unsaved data - it's gone already), but it does refresh the page and jump to the top again.
Before After
async-diff-view-switch-create-mr-before async-diff-view-switch-create-mr-after

How to set up and validate locally

  1. Begin the process to create a new MR from a branch that has changes
  2. Click to the "Changes" tab of the Create MR page.
  3. Click either Inline or Side-by-side to change the view type

Reviewing

It may be helpful to review this MR commit-by-commit, since the logic is scattered around the files.

Each commit is (mostly) very small parts of the larger change.

  • I've added the first tests for the diff.js Class
    • Only its construction and the new click handler are tested (these are what I modified)
  • Note the commit description for the merge_request_tabs_spec.js changes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Thomas Randolph

Merge request reports