Merge pull request #1992 from overleaf/pr-history-file-selection-improvements

History file selection and scrolling improvements

GitOrigin-RevId: e5f31eca07a72b25245f8883aec1664c320c0788

services/web/app/views/project/editor/history/entriesListV2.pug

@@ -101,6 +101,8 @@ script(type="text/ng-template", id="historyEntriesListTpl")
 				strong #{translate("ask_proj_owner_to_upgrade_for_full_history")}
 
 script(type="text/ng-template", id="historyEntryTpl")
+	time.history-entry-day(ng-if="::$ctrl.entry.meta.first_in_day") {{ ::$ctrl.entry.meta.end_ts | relativeDate }}
+
 	.history-entry(
 		ng-class="{\
 			'history-entry-first-in-day': $ctrl.entry.meta.first_in_day,\
@@ -115,9 +117,6 @@ script(type="text/ng-template", id="historyEntryTpl")
 		history-droppable-area-on-drop="$ctrl.onDrop(boundary)"
 		history-droppable-area-on-over="$ctrl.onOver(boundary)"
 	)
-
-		time.history-entry-day(ng-if="::$ctrl.entry.meta.first_in_day") {{ ::$ctrl.entry.meta.end_ts | relativeDate }}
-
 		.history-entry-details(
 			ng-click="$ctrl.onSelect({ selectedEntry: $ctrl.entry })"
 		)

services/web/public/src/ide/history/HistoryV2Manager.js

@@ -342,45 +342,113 @@ define([
         let files = this.$scope.history.selection.files
         let fileToSelect = null
         let previouslySelectedFile = null
+        let previouslySelectedFileHasOp = false
+        let filesWithOps = this._getFilesWithOps()
         const orderedOpTypes = ['edited', 'added', 'renamed', 'removed']
 
         if (this._previouslySelectedPathname != null) {
           previouslySelectedFile = _.find(files, {
             pathname: this._previouslySelectedPathname
           })
+          previouslySelectedFileHasOp = _.some(filesWithOps, {
+            pathname: this._previouslySelectedPathname
+          })
         }
-
-        if (previouslySelectedFile != null) {
+        if (previouslySelectedFile != null && previouslySelectedFileHasOp) {
           fileToSelect = previouslySelectedFile
         } else {
           for (let opType of orderedOpTypes) {
-            let fileWithMatchingOpType = _.find(files, { operation: opType })
+            let fileWithMatchingOpType = _.find(filesWithOps, {
+              operation: opType
+            })
             if (fileWithMatchingOpType != null) {
-              fileToSelect = fileWithMatchingOpType
+              fileToSelect = _.find(files, {
+                pathname: fileWithMatchingOpType.pathname
+              })
               break
             }
           }
           if (fileToSelect == null) {
-            let mainFile = _.find(files, function(file) {
-              return /main\.tex$/.test(file.pathname)
-            })
-            if (mainFile != null) {
-              fileToSelect = mainFile
+            if (previouslySelectedFile != null) {
+              fileToSelect = previouslySelectedFile
             } else {
-              let anyTeXFile = _.find(files, function(file) {
-                return /\.tex$/.test(file.pathname)
+              let mainFile = _.find(files, function(file) {
+                return /main\.tex$/.test(file.pathname)
               })
-              if (anyTeXFile != null) {
-                fileToSelect = anyTeXFile
+              if (mainFile != null) {
+                fileToSelect = mainFile
               } else {
-                fileToSelect = files[0]
+                let anyTeXFile = _.find(files, function(file) {
+                  return /\.tex$/.test(file.pathname)
+                })
+                if (anyTeXFile != null) {
+                  fileToSelect = anyTeXFile
+                } else {
+                  fileToSelect = files[0]
+                }
               }
             }
           }
         }
+
         this.selectFile(fileToSelect)
       }
 
+      _getFilesWithOps() {
+        let filesWithOps
+        if (this.$scope.history.viewMode === HistoryViewModes.POINT_IN_TIME) {
+          let currentUpdate = this.getUpdateForVersion(
+            this.$scope.history.selection.range.toV
+          )
+          filesWithOps = []
+          if (currentUpdate != null) {
+            for (pathname of currentUpdate.pathnames) {
+              filesWithOps.push({
+                pathname: pathname,
+                operation: 'edited'
+              })
+            }
+            for (op of currentUpdate.project_ops) {
+              let fileWithOp
+              if (op.add != null) {
+                fileWithOp = {
+                  pathname: op.add.pathname,
+                  operation: 'added'
+                }
+              } else if (op.remove != null) {
+                fileWithOp = {
+                  pathname: op.remove.pathname,
+                  operation: 'removed'
+                }
+              } else if (op.rename != null) {
+                fileWithOp = {
+                  pathname: op.rename.newPathname,
+                  operation: 'renamed'
+                }
+              }
+              if (fileWithOp != null) {
+                filesWithOps.push(fileWithOp)
+              }
+            }
+          }
+        } else {
+          filesWithOps = _.reduce(
+            this.$scope.history.selection.files,
+            (curFilesWithOps, file) => {
+              if (file.operation) {
+                curFilesWithOps.push({
+                  pathname: file.pathname,
+                  operation: file.operation
+                })
+              }
+              return curFilesWithOps
+            },
+            []
+          )
+        }
+        return filesWithOps
+      }
+
       autoSelectRecentUpdates() {
         if (this.$scope.history.updates.length === 0) {
           return

services/web/public/stylesheets/app/editor/history-v2.less

@@ -81,6 +81,9 @@
 	color: #FFF;
 	padding: 5px 10px;
 	line-height: 1;
+	position: sticky;
+	top: 0;
+	z-index: 3;
 }
 
 .history-entry-toV-handle,

services/web/test/unit_frontend/src/ide/history/HistoryV2ManagerTests.js

@@ -215,124 +215,232 @@ define(['ide/history/HistoryV2Manager'], HistoryV2Manager =>
     })
 
     describe('autoSelectFile', function() {
-      beforeEach(function() {
-        this.mockedFilesList = [
-          {
-            pathname: 'main.tex'
-          },
-          {
-            pathname: 'references.bib'
-          },
-          {
-            pathname: 'universe.jpg'
-          },
-          {
-            pathname: 'chapters/chapter2.tex'
-          },
-          {
-            pathname: 'chapters/draft.tex',
-            operation: 'removed',
-            deletedAtV: 47
-          },
-          {
-            pathname: 'chapters/chapter3.tex',
-            operation: 'added'
-          },
-          {
-            pathname: 'chapters/chapter1.tex',
-            operation: 'edited'
-          },
-          {
-            pathname: 'chapters/foo.tex',
-            oldPathname: 'chapters/bar.tex',
-            operation: 'renamed'
-          }
-        ]
-        this.mockedMainTex = this.mockedFilesList[0]
-        this.mockedReferencesFile = this.mockedFilesList[1]
-        this.mockedRemovedFile = this.mockedFilesList[4]
-        this.mockedAddedFile = this.mockedFilesList[5]
-        this.mockedEditedFile = this.mockedFilesList[6]
-        this.mockedRenamedFile = this.mockedFilesList[7]
-
-        this.$scope.history.selection.files = this.mockedFilesList
-      })
+      describe('for compare mode', function() {
+        beforeEach(function() {
+          this.mockedFilesList = [
+            {
+              pathname: 'main.tex'
+            },
+            {
+              pathname: 'references.bib'
+            },
+            {
+              pathname: 'universe.jpg'
+            },
+            {
+              pathname: 'chapters/chapter2.tex'
+            },
+            {
+              pathname: 'chapters/draft.tex',
+              operation: 'removed',
+              deletedAtV: 47
+            },
+            {
+              pathname: 'chapters/chapter3.tex',
+              operation: 'added'
+            },
+            {
+              pathname: 'chapters/chapter1.tex',
+              operation: 'edited'
+            },
+            {
+              pathname: 'chapters/foo.tex',
+              oldPathname: 'chapters/bar.tex',
+              operation: 'renamed'
+            }
+          ]
+          this.mockedMainTex = this.mockedFilesList[0]
+          this.mockedReferencesFile = this.mockedFilesList[1]
+          this.mockedRemovedFile = this.mockedFilesList[4]
+          this.mockedAddedFile = this.mockedFilesList[5]
+          this.mockedEditedFile = this.mockedFilesList[6]
+          this.mockedRenamedFile = this.mockedFilesList[7]
 
-      describe('with a previously selected file', function() {
-        it('should prefer the previously selected file if it is available', function() {
-          this.historyManager._previouslySelectedPathname = this.mockedReferencesFile.pathname
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedReferencesFile
-          )
+          this.$scope.history.viewMode = 'compare'
+          this.$scope.history.selection.files = this.mockedFilesList
         })
 
-        it('should ignore the previously selected file if it is not available', function() {
-          this.historyManager._previouslySelectedPathname = 'non/existent.file'
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file.pathname).to.not.equal(
-            'non/existent.file'
-          )
-        })
-      })
+        describe('with a previously selected file', function() {
+          it('should prefer the previously selected file if it is available and has operations', function() {
+            this.historyManager._previouslySelectedPathname = this.mockedAddedFile.pathname
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedAddedFile
+            )
+          })
 
-      describe('without a previously selected file, with a list of files containing operations', function() {
-        it('should prefer edited files', function() {
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedEditedFile
-          )
-        })
+          it('should prefer a file with ops if the previously selected file is available but has no operations', function() {
+            this.historyManager._previouslySelectedPathname = this.mockedReferencesFile.pathname
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.operation).to.exist
+          })
 
-        it('should prefer added files if no edited files are present', function() {
-          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedEditedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedAddedFile
-          )
+          it('should ignore the previously selected file if it is not available', function() {
+            this.historyManager._previouslySelectedPathname =
+              'non/existent.file'
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.not.equal(
+              'non/existent.file'
+            )
+          })
         })
 
-        it('should prefer renamed files if no edited or added files are present', function() {
-          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedEditedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
-          const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedAddedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedRenamedFile
-          )
+        describe('without a previously selected file, with a list of files containing operations', function() {
+          it('should prefer edited files', function() {
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedEditedFile
+            )
+          })
+
+          it('should prefer added files if no edited files are present', function() {
+            const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedEditedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedAddedFile
+            )
+          })
+
+          it('should prefer renamed files if no edited or added files are present', function() {
+            const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedEditedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+            const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedAddedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedRenamedFile
+            )
+          })
+
+          it('should prefer removed files if no edited, added or renamed files are present', function() {
+            const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedEditedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+            const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedAddedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
+            const indexOfRenamedFile = this.$scope.history.selection.files.indexOf(
+              this.mockedRenamedFile
+            )
+            this.$scope.history.selection.files.splice(indexOfRenamedFile, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedRemovedFile
+            )
+          })
         })
 
-        it('should prefer removed files if no edited, added or renamed files are present', function() {
-          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedEditedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
-          const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedAddedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
-          const indexOfRenamedFile = this.$scope.history.selection.files.indexOf(
-            this.mockedRenamedFile
-          )
-          this.$scope.history.selection.files.splice(indexOfRenamedFile, 1)
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedRemovedFile
-          )
+        describe('without a previously selected file, with a list of files without operations', function() {
+          beforeEach(function() {
+            this.mockedFilesListWithNoOps = [
+              {
+                pathname: 'main.tex'
+              },
+              {
+                pathname: 'references.bib'
+              },
+              {
+                pathname: 'other.tex'
+              },
+              {
+                pathname: 'universe.jpg'
+              }
+            ]
+            this.mockedMainTex = this.mockedFilesListWithNoOps[0]
+            this.mockedReferencesFile = this.mockedFilesListWithNoOps[1]
+            this.mockedOtherTexFile = this.mockedFilesListWithNoOps[2]
+
+            this.$scope.history.selection.files = this.mockedFilesListWithNoOps
+          })
+
+          it('should prefer main.tex if it exists', function() {
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedMainTex
+            )
+          })
+
+          it('should prefer another tex file if main.tex does not exist', function() {
+            const indexOfMainTex = this.$scope.history.selection.files.indexOf(
+              this.mockedMainTex
+            )
+            this.$scope.history.selection.files.splice(indexOfMainTex, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedOtherTexFile
+            )
+          })
+
+          it('should pick the first available file if no tex files are available', function() {
+            const indexOfMainTex = this.$scope.history.selection.files.indexOf(
+              this.mockedMainTex
+            )
+            this.$scope.history.selection.files.splice(indexOfMainTex, 1)
+            const indexOfOtherTexFile = this.$scope.history.selection.files.indexOf(
+              this.mockedOtherTexFile
+            )
+            this.$scope.history.selection.files.splice(indexOfOtherTexFile, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file).to.deep.equal(
+              this.mockedReferencesFile
+            )
+          })
         })
       })
 
-      describe('without a previously selected file, with a list of files without operations', function() {
+      describe('for point-in-time mode', function() {
         beforeEach(function() {
-          this.mockedFilesListWithNoOps = [
+          this.$scope.history.viewMode = 'point_in_time'
+          this.sampleUpdates[0] = {
+            fromV: 4,
+            toV: 5,
+            meta: {
+              users: [
+                {
+                  first_name: 'john.doe',
+                  last_name: '',
+                  email: 'john.doe@domain.tld',
+                  id: '5b57299087712202fb599ab4',
+                  hue: 200
+                }
+              ],
+              start_ts: 1544021278346,
+              end_ts: 1544021278346
+            },
+            pathnames: ['main.tex'],
+            project_ops: [
+              {
+                add: {
+                  pathname: 'chapters/chapter1.tex'
+                },
+                atV: 4
+              },
+              {
+                rename: {
+                  pathname: 'foo.tex',
+                  newPathname: 'bar.tex'
+                }
+              }
+            ]
+          }
+          this.sampleUpdateEditedFile = this.sampleUpdates[0].pathnames[0]
+          this.sampleUpdateAddedFile = this.sampleUpdates[0].project_ops[0].add.pathname
+          this.sampleUpdateRenamedFile = this.sampleUpdates[0].project_ops[1].rename.newPathname
+          this.$scope.history.updates = this.sampleUpdates
+          this.$scope.history.selection.range = {
+            fromV: this.sampleUpdates[0].toV,
+            toV: this.sampleUpdates[0].toV
+          }
+          this.mockedFilesList = [
             {
               pathname: 'main.tex'
             },
@@ -340,50 +448,106 @@ define(['ide/history/HistoryV2Manager'], HistoryV2Manager =>
               pathname: 'references.bib'
             },
             {
-              pathname: 'other.tex'
+              pathname: 'universe.jpg'
             },
             {
-              pathname: 'universe.jpg'
+              pathname: 'chapters/chapter2.tex'
+            },
+            {
+              pathname: 'chapters/draft.tex'
+            },
+            {
+              pathname: 'chapters/chapter3.tex'
+            },
+            {
+              pathname: 'chapters/chapter1.tex'
+            },
+            {
+              pathname: 'bar.tex'
             }
           ]
-          this.mockedMainTex = this.mockedFilesListWithNoOps[0]
-          this.mockedReferencesFile = this.mockedFilesListWithNoOps[1]
-          this.mockedOtherTexFile = this.mockedFilesListWithNoOps[2]
-
-          this.$scope.history.selection.files = this.mockedFilesListWithNoOps
+          this.$scope.history.selection.files = this.mockedFilesList
         })
 
-        it('should prefer main.tex if it exists', function() {
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedMainTex
-          )
+        describe('with a previously selected file', function() {
+          it('should prefer the previously selected file if it is available and has operations', function() {
+            this.historyManager._previouslySelectedPathname = 'main.tex'
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.sampleUpdateEditedFile
+            )
+          })
+
+          it('should prefer a file with ops if the previously selected file is available but has no operations', function() {
+            this.historyManager._previouslySelectedPathname = 'main.tex'
+            this.sampleUpdates[0].pathnames = []
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.sampleUpdateAddedFile
+            )
+          })
+
+          it('should ignore the previously selected file if it is not available', function() {
+            this.historyManager._previouslySelectedPathname =
+              'non/existent.file'
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.not.equal(
+              'non/existent.file'
+            )
+          })
         })
 
-        it('should prefer another tex file if main.tex does not exist', function() {
-          const indexOfMainTex = this.$scope.history.selection.files.indexOf(
-            this.mockedMainTex
-          )
-          this.$scope.history.selection.files.splice(indexOfMainTex, 1)
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedOtherTexFile
-          )
+        describe('without a previously selected file, with a list of files containing operations', function() {
+          it('should prefer edited files', function() {
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.sampleUpdateEditedFile
+            )
+          })
+
+          it('should prefer added files if no edited files are present', function() {
+            this.sampleUpdates[0].pathnames = []
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.sampleUpdateAddedFile
+            )
+          })
+
+          it('should prefer renamed files if no edited or added files are present', function() {
+            this.sampleUpdates[0].pathnames = []
+            this.sampleUpdates[0].project_ops.shift()
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.sampleUpdateRenamedFile
+            )
+          })
         })
 
-        it('should pick the first available file if no tex files are available', function() {
-          const indexOfMainTex = this.$scope.history.selection.files.indexOf(
-            this.mockedMainTex
-          )
-          this.$scope.history.selection.files.splice(indexOfMainTex, 1)
-          const indexOfOtherTexFile = this.$scope.history.selection.files.indexOf(
-            this.mockedOtherTexFile
-          )
-          this.$scope.history.selection.files.splice(indexOfOtherTexFile, 1)
-          this.historyManager.autoSelectFile()
-          expect(this.$scope.history.selection.file).to.deep.equal(
-            this.mockedReferencesFile
-          )
+        describe('without a previously selected file, with a list of files without operations', function() {
+          beforeEach(function() {
+            this.sampleUpdates[0].pathnames = []
+            this.sampleUpdates[0].project_ops = []
+            this.mockedMainTex = this.mockedFilesList[0]
+            this.mockedReferencesFile = this.mockedFilesList[1]
+          })
+
+          it('should prefer main.tex if it exists', function() {
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.equal(
+              this.mockedMainTex.pathname
+            )
+          })
+
+          it('should prefer another tex file if main.tex does not exist', function() {
+            const indexOfMainTex = this.$scope.history.selection.files.indexOf(
+              this.mockedMainTex
+            )
+            this.$scope.history.selection.files.splice(indexOfMainTex, 1)
+            this.historyManager.autoSelectFile()
+            expect(this.$scope.history.selection.file.pathname).to.match(
+              /.tex$/
+            )
+          })
         })
       })
     })