[go: up one dir, main page]

Skip to content

Differentiate between automatically collapsed and manually (user) collapsed diff files

In the course of two other issues relating to collapsed files, it became apparent that we need a way to track when the user collapses a diff file versus when that file started collapsed (whether for performance reasons, or the content didn't change, etc.).

In !41393 (merged), we shouldn't show the big blaring warning if the user has already loaded the full file and manually collapsed it or if the file was fully loaded to begin with and the user collapsed that file during the course of their normal review process.

In !40752 (merged), we shouldn't show the big warning at the top of the page (soon to be sticky!) if the user manually collapses a file (in all the same scenarios). In that MR, there's currently a rough check if the user has actively dismissed the alert to keep it from coming back if the user has acknowledged it.

Both of these cases need a more formalized distinction between files that were collapsed by algorithms and files that were collapsed by the user.

Rough suggestion

  1. Update the current viewer.collapsed property to viewer.automaticallyCollapsed
  2. Add viewer.manuallyCollapsed (and all of the Vuex boilerplate to toggle that value)
  3. Add a getter for an individual file that resolves all of the possible logic values (automaticallyCollapsed, manuallyCollapsed, etc.) to the correct isCollapsed value for the diff_file component.
    • There's more than just collapsed values here. Sometimes, a file isn't expanded if it's "unreadable text" or if there's an error. These perhaps shouldn't be included in an isCollapsed computation, but they're part of the logic that determines if the body should show. It would be nice to centralize that (ever expanding) logic somehow.
  4. Update the diff_file component to toggle manuallyCollapsed
  5. Update the diff_file component to display the correct body based on isCollapsed plus manuallyCollapsed/automaticallyCollapsed