Skip to content

File-by-file: Add action for async scrollToFile

What does this MR do and why?

This change will ensure that when we try to scroll to a file, it should always be present before the scroll occurs.

Why

Context

For #337263 (closed).

We are making File-by-file view mode much more useful by only fetching the single file that you are trying to view at any time.

In extreme cases, if the file you are viewing is the last in a long list (say 100s or 1000s of files), batch-loading the diffs could introduce an extremely noticeable delay before your single file is loaded and displays while the app loads tons of files you have no interest in.

The above issue - and this chain of MRs - will resolve that problem by loading only the file needed to display.

Specifics

Before, we always had a full representation of the diff, so calling scrollToFile would generally work.

(I have questions about when the file tree has loaded or the URL has been parsed, but the file in question is still being loaded by a later batch).

However, we are now going to implement on-demand single-file loading, which means clicking on a file tree entry (as one example) may ALWAYS try to scroll to a file that doesn't exist yet.

This new action (goToFile) wraps the previous action to reduce API churn / branching logic and - if the view is not in file-by-file mode, just immediately defers to the prior scrollToFile. However, if the file hasn't yet been loaded, and we are in file-by-file mode, it first triggers a fetch for that file, and when that's done then triggers the scroll.

This MR is blocked by !114009 (merged), but "blocking MR cannot itself be blocked" for reasons unknown.

Roadmap

This MR is one of a set of changes that will eventually implement this feature.

Phase 1 - Preparation

Status MR Purpose
🚀 Merged !112515 (merged) Adds the API access we need for the more surgical data
🚀 Merged !112516 (merged) Switches to using our initial metadata representation of the MR for many operations
🚀 Merged !113941 (merged) Marks metadata files as loaded so future uses don't try to load them again

Phase 2 - New Abilities

Status MR Purpose
🚀 Merged !113940 (merged) Adds helpers to re-retrieve the hash from the URL since it only needed to be fetched once before
🚀 Merged !113937 (merged) Adds an action to fetch and store a single file versus the existing full diff batch loading
🚀 Merged !114009 (merged) Add a getter to check the files marked in !113941 (merged) to simplify "already-loaded" guard clauses
We are here 👉🏻 !114024 (merged) Add an action to scroll to a file async (and load it if necessary)
🚀 Merged !114170 (merged) Adds an action to re-check the note hash from the URL and potentially fetch the appropriate file

Phase 3 - Implementation

Status MR Purpose
🛠 In Dev !111895 (merged) Adds the feature that will only load the proper file when necessary to display it

Screenshots or screen recordings

N/A, all backstage

How to set up and validate locally

Since this code isn't yet used, there's nothing to validate other than running the unit tests.

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