Migrate changes to JLab1.0 and latest NBDime

Jaipreet Singh · Jaipreet Singh · commit 1f1ebd1f9960 · 2019-07-31T21:44:17.000-07:00
* The update to nbdime5.x lost us some styles. This change adds them back and also makes some minor UI tweaks to the collapse/expand button.
* Add back conditionally adding the commands since adding the commands more than once leads to errors when the file panel is re-rendered.
* Update tests based on the latest changes in the NBDime API.
diff --git a/package.json b/package.json
@@ -48,6 +48,7 @@
     "@jupyterlab/services": "^4.0.0",
     "@jupyterlab/terminal": "^1.0.0",
     "@phosphor/widgets": "^1.8.0",
+    "nbdime": "~5.0.1",
     "react": "~16.8.4",
     "react-dom": "~16.8.4",
     "typestyle": "^2.0.1"
diff --git a/src/components/FileList.tsx b/src/components/FileList.tsx
@@ -110,109 +110,115 @@ export class FileList extends React.Component<IFileListProps, IFileListState> {
       disableFiles: false
     };
 
-    commands.addCommand(CommandIDs.gitFileOpen, {
-      label: 'Open',
-      caption: 'Open selected file',
-      execute: async () => {
-        await this.openListedFile(
-          this.state.contextMenuTypeX,
-          this.state.contextMenuTypeY,
-          this.state.contextMenuFile,
-          this.props.app
-        );
-      }
-    });
+    if (!commands.hasCommand(CommandIDs.gitFileOpen)) {
+      commands.addCommand(CommandIDs.gitFileOpen, {
+        label: 'Open',
+        caption: 'Open selected file',
+        execute: async () => {
+          await this.openListedFile(
+            this.state.contextMenuTypeX,
+            this.state.contextMenuTypeY,
+            this.state.contextMenuFile,
+            this.props.app
+          );
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileDiffWorking, {
-      label: 'Diff',
-      caption: 'Diff selected file',
-      execute: () => {
-        openDiffView(
-          this.state.contextMenuFile,
-          this.props.app,
-          {
-            currentRef: { specialRef: 'WORKING' },
-            previousRef: { gitRef: 'HEAD' }
-          },
-          this.props.renderMime
-        );
-      }
-    });
+    if (!commands.hasCommand(CommandIDs.gitFileDiffWorking)) {
+      commands.addCommand(CommandIDs.gitFileDiffWorking, {
+        label: 'Diff',
+        caption: 'Diff selected file',
+        execute: () => {
+          openDiffView(
+            this.state.contextMenuFile,
+            this.props.app,
+            {
+              currentRef: { specialRef: 'WORKING' },
+              previousRef: { gitRef: 'HEAD' }
+            },
+            this.props.renderMime
+          );
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileDiffIndex, {
-      label: 'Diff',
-      caption: 'Diff selected file',
-      execute: () => {
-        openDiffView(
-          this.state.contextMenuFile,
-          this.props.app,
-          {
-            currentRef: { specialRef: 'INDEX' },
-            previousRef: { gitRef: 'HEAD' }
-          },
-          this.props.renderMime
-        );
-      }
-    });
+    if (!commands.hasCommand(CommandIDs.gitFileDiffIndex)) {
+      commands.addCommand(CommandIDs.gitFileDiffIndex, {
+        label: 'Diff',
+        caption: 'Diff selected file',
+        execute: () => {
+          openDiffView(
+            this.state.contextMenuFile,
+            this.props.app,
+            {
+              currentRef: { specialRef: 'INDEX' },
+              previousRef: { gitRef: 'HEAD' }
+            },
+            this.props.renderMime
+          );
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileStage, {
-      label: 'Stage',
-      caption: 'Stage the changes of selected file',
-      execute: () => {
-        try {
+    if (!commands.hasCommand(CommandIDs.gitFileStage)) {
+      commands.addCommand(CommandIDs.gitFileStage, {
+        label: 'Stage',
+        caption: 'Stage the changes of selected file',
+        execute: () => {
           this.addUnstagedFile(
             this.state.contextMenuFile,
             this.props.topRepoPath,
             this.props.refresh
           );
-        } catch (err) {}
-      }
-    });
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileTrack, {
-      label: 'Track',
-      caption: 'Start tracking selected file',
-      execute: () => {
-        try {
+    if (!commands.hasCommand(CommandIDs.gitFileTrack)) {
+      commands.addCommand(CommandIDs.gitFileTrack, {
+        label: 'Track',
+        caption: 'Start tracking selected file',
+        execute: () => {
           this.addUntrackedFile(
             this.state.contextMenuFile,
             this.props.topRepoPath,
             this.props.refresh
           );
-        } catch (err) {}
-      }
-    });
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileUnstage, {
-      label: 'Unstage',
-      caption: 'Unstage the changes of selected file',
-      execute: () => {
-        try {
+    if (!commands.hasCommand(CommandIDs.gitFileUnstage)) {
+      commands.addCommand(CommandIDs.gitFileUnstage, {
+        label: 'Unstage',
+        caption: 'Unstage the changes of selected file',
+        execute: () => {
           if (this.state.contextMenuTypeX !== 'D') {
             this.resetStagedFile(
               this.state.contextMenuFile,
               this.props.topRepoPath,
               this.props.refresh
             );
           }
-        } catch (err) {}
-      }
-    });
+        }
+      });
+    }
 
-    commands.addCommand(CommandIDs.gitFileDiscard, {
-      label: 'Discard',
-      caption: 'Discard recent changes of selected file',
-      execute: () => {
-        try {
+    if (!commands.hasCommand(CommandIDs.gitFileDiscard)) {
+      commands.addCommand(CommandIDs.gitFileDiscard, {
+        label: 'Discard',
+        caption: 'Discard recent changes of selected file',
+        execute: () => {
           this.updateSelectedFile(
             this.state.contextMenuIndex,
             this.state.contextMenuStage
           );
           this.updateSelectedDiscardFile(this.state.contextMenuIndex);
           this.toggleDisableFiles();
-        } catch (err) {}
-      }
-    });
+        }
+      });
+    }
 
     this.state.contextMenuStaged.addItem({ command: CommandIDs.gitFileOpen });
     this.state.contextMenuStaged.addItem({
@@ -328,6 +334,7 @@ export class FileList extends React.Component<IFileListProps, IFileListState> {
   updateSelectedStage = (stage: string): void => {
     this.setState({ selectedStage: stage });
   };
+
   /** Open a file in the git listing */
   async openListedFile(
     typeX: string,
diff --git a/src/components/diff/DiffWidget.tsx b/src/components/diff/DiffWidget.tsx
@@ -5,7 +5,7 @@ import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
 
 import { getRefValue, IDiffContext } from './model';
 import { Diff, isDiffSupported, RenderMimeProvider } from './Diff';
-import { JupyterLab } from '@jupyterlab/application';
+import { JupyterFrontEnd } from '@jupyterlab/application';
 import { showDialog } from '@jupyterlab/apputils';
 import { PathExt } from '@jupyterlab/coreutils';
 import { style } from 'typestyle';
@@ -62,7 +62,7 @@ export class DiffWidget extends Widget {
  */
 export function openDiffView(
   path: string,
-  app: JupyterLab,
+  app: JupyterFrontEnd,
   diffContext: IDiffContext,
   renderMime: IRenderMimeRegistry
 ) {
@@ -80,7 +80,7 @@ export function openDiffView(
     if (!mainAreaItem) {
       const nbDiffWidget = new DiffWidget(renderMime, path, diffContext);
       nbDiffWidget.id = id;
-      app.shell.addToMainArea(nbDiffWidget);
+      app.shell.add(nbDiffWidget, 'main');
       app.shell.activateById(nbDiffWidget.id);
     }
   } else {
diff --git a/src/components/diff/NbDiff.tsx b/src/components/diff/NbDiff.tsx
@@ -173,8 +173,8 @@ export class NBDiff extends React.Component<IDiffProps, INBDiffState> {
 
       httpGitRequest('/nbdime/api/gitdiff', 'POST', {
         file_path: this.props.path,
-        ref_prev: { git: diffContext.previousRef.gitRef },
-        ref_curr: currentRefValue
+        ref_local: { git: diffContext.previousRef.gitRef },
+        ref_remote: currentRefValue
       }).then((response: Response) => {
         response
           .json()
diff --git a/src/componentsStyle/GitStageStyle.tsx b/src/componentsStyle/GitStageStyle.tsx
@@ -76,12 +76,13 @@ export const changeStageButtonStyle = style({
 
 export const discardFileButtonStyle = style({
   backgroundImage: 'var(--jp-icon-discard-file)',
-  marginLeft: '6px'
+  marginLeft: '6px',
+  paddingRight: '1px'
 });
 
 export const diffFileButtonStyle = style({
   backgroundImage: 'var(--jp-icon-diff)',
-  marginLeft: '6px'
+  paddingLeft: '0px'
 });
 
 export const caretdownImageStyle = style({
diff --git a/style/diff.css b/style/diff.css
@@ -76,7 +76,7 @@
     background-size: 20px;
     background-position: center;
     background-repeat: no-repeat;
-    height: 15px;
+    height: 10px;
     width: 15px;
     font-size: var(--jp-ui-font-size1);
     line-height: var(--jp-private-toolbar-height);
@@ -87,6 +87,7 @@
     text-decoration: none;
     transition: background-color 0.1s ease;
     margin-left: 5px;
+    border: transparent;
 }
 
 .jp-git-diff-Widget {
@@ -452,3 +453,56 @@
 .jp-git-Notebook-diff .jp-Diff-renderedOutput {
     min-height: 2em;
 }
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel {
+    border: var(--jp-git-diff-output-border-color) solid 1pt;
+}
+
+.jp-git-Notebook-diff button.jp-CollapsiblePanel-header-icon::-moz-focus-inner {
+    border: none;
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-header-icon-opened {
+    background-image: var(--jp-icon-caretup);
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-header-icon-closed {
+    background-image: var(--jp-icon-caretdown);
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-header {
+    padding-top: .2em;
+    padding-bottom: .2em;
+    background-color: var(--jp-git-diff-output-color);
+    padding-left: 2em;
+    padding-right: 2em;
+    color: var(--jp-ui-font-color1);
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-container {
+    overflow: hidden;
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-slider {
+    height:100%;
+    -webkit-transition: -webkit-transform .3s ease, height 0.3s;
+    -moz-transition:    -moz-transform .3s ease, height 0.3s;
+    -ms-transition:     -ms-transform .3s ease, height 0.3s;
+    transition:         transform .3s ease, height 0.3s;
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-opened {
+    -webkit-transform: translate(0, 0%);
+    -moz-transform: translate(0, 0%);
+    -ms-transform: translate(0, 0%);
+    transform: translate(0, 0%);
+    height: 100%;
+}
+
+.jp-git-Notebook-diff .jp-CollapsiblePanel-closed {
+    -webkit-transform: translate(0, -100%);
+    -moz-transform: translate(0, -100%);
+    -ms-transform: translate(0, -100%);
+    transform: translate(0, -100%);
+    height: 0;
+}
diff --git a/style/variables.css b/style/variables.css
@@ -21,6 +21,8 @@
   --jp-icon-git-pull-disabled: url('./images/git-clone-icon-disabled.svg');
   --jp-git-diff-deleted-color: rgba(255, 193, 193, 0.5);
   --jp-git-diff-added-color: rgba(201, 243, 194, 0.5);
+  --jp-git-diff-output-color: rgba(0, 141, 255, 0.3);
+  --jp-git-diff-output-border-color: rgba(0, 141, 255, 0.7);
 }
 
 [data-jp-theme-light='true'] {
diff --git a/tests/test-components/NBDiff.spec.tsx b/tests/test-components/NBDiff.spec.tsx
@@ -38,8 +38,8 @@ describe('NBDiff', () => {
       expect(httpGitRequest).toHaveBeenCalled();
       expect(httpGitRequest).toBeCalledWith('/nbdime/api/gitdiff', 'POST', {
         file_path: '/path/to/File.ipynb',
-        ref_curr: { special: 'WORKING' },
-        ref_prev: { git: '83baee' }
+        ref_remote: { special: 'WORKING' },
+        ref_local: { git: '83baee' }
       });
       expect(node.find('.jp-git-diff-error').text()).toContain(
         'TEST_ERROR_MESSAGE'
@@ -75,8 +75,8 @@ describe('NBDiff', () => {
       expect(httpGitRequest).toHaveBeenCalled();
       expect(httpGitRequest).toBeCalledWith('/nbdime/api/gitdiff', 'POST', {
         file_path: '/path/to/File.ipynb',
-        ref_curr: { special: 'WORKING' },
-        ref_prev: { git: '83baee' }
+        ref_remote: { special: 'WORKING' },
+        ref_local: { git: '83baee' }
       });
       expect(node.find('.jp-git-diff-error').html()).toBeNull();
       expect(node.find(NBDiffHeader)).toHaveLength(1);
@@ -99,13 +99,13 @@ function createTestResponse(
     trailer: Promise.resolve(Headers as any),
     type: 'basic',
     url: '/foo/bar',
-    clone: jest.fn<Response>(),
+    clone: jest.fn<Response, []>(),
     body: null,
     bodyUsed: false,
-    arrayBuffer: jest.fn<Promise<ArrayBuffer>>(),
-    blob: jest.fn<Promise<Blob>>(),
-    formData: jest.fn<Promise<FormData>>(),
-    text: jest.fn<Promise<String>>()
+    arrayBuffer: jest.fn<Promise<ArrayBuffer>, []>(),
+    blob: jest.fn<Promise<Blob>, []>(),
+    formData: jest.fn<Promise<FormData>, []>(),
+    text: jest.fn<Promise<string>, []>()
   };
   return Promise.resolve(testResponse);
 }
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,8 @@`
`21`	`21`	`--jp-icon-git-pull-disabled: url('./images/git-clone-icon-disabled.svg');`
`22`	`22`	`--jp-git-diff-deleted-color: rgba(255, 193, 193, 0.5);`
`23`	`23`	`--jp-git-diff-added-color: rgba(201, 243, 194, 0.5);`
	`24`	`+ --jp-git-diff-output-color: rgba(0, 141, 255, 0.3);`
	`25`	`+ --jp-git-diff-output-border-color: rgba(0, 141, 255, 0.7);`
`24`	`26`	`}`
`25`	`27`
`26`	`28`	`[data-jp-theme-light='true'] {`