[go: up one dir, main page]
Skip to content

Merge Request "Show one file at a time" is unable to handle browser and unresolved thread navigation

Summary

In Merge Request Changes tab, the "Show one file at a time" setting breaks browser back and forward + unresolved thread navigation. This relates to #403084 (closed) (not caused by it, but rather symptoms of the same problem it was attempting to solve)

Steps to reproduce

Start with !101724 (diffs)

  1. Enable "Show one file at a time" and click on any file, e.g., !101724 (diffs).
  2. Click on another file, e.g., !101724 (diffs).
  3. Attempt to use the browser back and forward button. Note that while the url properly changes to show the relevant file fragments (and there's also some visual movement), the displayed file remains the same.

Similarly, start with a a merge request with unresolved threads

  1. Enable "Show one file at a time".
  2. Navigate to a file that has no unresolved threads.
  3. Attempt to use the unresolved thread navigator. You're taken to the overview page view of them.
  4. Go back to a file that has unresolved threads and attempt to use the unresolved thread navigator. Nothing happens.
  5. This is as opposed to the non "Show one file at a time" view that properly navigates between the unresolved threads in the diff view.

Example Project

See steps to reproduce, this applies on any merge request

What is the current bug behavior?

"Show one file at a time" in merge request diff view prevents browser back and forward navigation between files. It also breaks unresolved thread navigation.

What is the expected correct behavior?

"Show one file at a time" in merge request diff view still allows browser back and forward navigation between files as well as unresolved thread navigation to work as if "Show one file at a time" wasn't enabled.

Relevant logs and/or screenshots

See reproduction steps for gitlab.com. Anyone can reproduce this.

Output of checks

N/A

Results of GitLab environment info

N/A

Possible fixes

I believe the fix around #403084 (closed) should work for this as well, forcing a fragment resolution and load when the UI refreshes. @thomasrandolph (sorry to tag you but I think you're the most relevant person to help here given the previous work, which we very much appreciate!)