From e1de27879904c3ff1f089a7ddbd5cc9ac8c719b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Sat, 26 Sep 2020 18:29:14 +0200 Subject: [PATCH] Virtualized files list (#767) * Virtualized files list * Windowing on history files list * Window on branches list * Handle filter branch list * Correct unit test --- jupyterlab_git/handlers.py | 3 + package.json | 4 + src/components/BranchMenu.tsx | 54 +- src/components/FileItem.tsx | 147 ++++-- src/components/FileList.tsx | 574 +++++++++++++--------- src/components/GitStage.tsx | 60 ++- src/components/NewBranchDialog.tsx | 66 ++- src/components/SinglePastCommitInfo.tsx | 67 +-- src/style/BranchMenu.ts | 10 - src/style/FileItemStyle.ts | 1 + src/style/FileListStyle.ts | 2 +- src/style/NewBranchDialog.ts | 9 +- src/style/PastCommitNode.ts | 2 +- tests/test-components/BranchMenu.spec.tsx | 33 +- tests/test-components/FileItem.spec.tsx | 3 +- yarn.lock | 32 ++ 16 files changed, 642 insertions(+), 425 deletions(-) diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index 6341cc6f7..e172549f5 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -175,6 +175,9 @@ async def post(self): selected_hash = data["selected_hash"] current_path = data["current_path"] result = await self.git.detailed_log(selected_hash, current_path) + + if result["code"] != 0: + self.set_status(500) self.finish(json.dumps(result)) diff --git a/package.json b/package.json index af93f8847..657ae4bda 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,8 @@ "react": "~16.9.0", "react-dom": "~16.9.0", "react-textarea-autosize": "^7.1.2", + "react-virtualized-auto-sizer": "^1.0.2", + "react-window": "^1.8.5", "typestyle": "^2.0.1" }, "devDependencies": { @@ -86,6 +88,8 @@ "@types/react": "~16.8.13", "@types/react-dom": "~16.0.5", "@types/react-textarea-autosize": "^4.3.5", + "@types/react-virtualized-auto-sizer": "^1.0.0", + "@types/react-window": "^1.8.2", "@typescript-eslint/eslint-plugin": "^2.25.0", "@typescript-eslint/parser": "^2.25.0", "all-contributors-cli": "^6.14.0", diff --git a/src/components/BranchMenu.tsx b/src/components/BranchMenu.tsx index 7262d66fe..6cf0ca79d 100644 --- a/src/components/BranchMenu.tsx +++ b/src/components/BranchMenu.tsx @@ -3,6 +3,7 @@ import List from '@material-ui/core/List'; import ListItem from '@material-ui/core/ListItem'; import ClearIcon from '@material-ui/icons/Clear'; import * as React from 'react'; +import { FixedSizeList, ListChildComponentProps } from 'react-window'; import { classes } from 'typestyle'; import { activeListItemClass, @@ -12,7 +13,6 @@ import { filterWrapperClass, listItemClass, listItemIconClass, - listWrapperClass, newBranchButtonClass, wrapperClass } from '../style/BranchMenu'; @@ -24,6 +24,9 @@ import { SuspendModal } from './SuspendModal'; const CHANGES_ERR_MSG = 'The current branch contains files with uncommitted changes. Please commit or discard these changes before switching to or creating another branch.'; +const ITEM_HEIGHT = 24.8; // HTML element height for a single branch +const MIN_HEIGHT = 150; // Minimal HTML element height for the branches list +const MAX_HEIGHT = 400; // Maximal HTML element height for the branches list /** * Callback invoked upon encountering an error when switching branches. @@ -237,37 +240,38 @@ export class BranchMenu extends React.Component< * @returns React element */ private _renderBranchList(): React.ReactElement { + // Perform a "simple" filter... (TODO: consider implementing fuzzy filtering) + const filter = this.state.filter; + const branches = this.state.branches.filter( + branch => !filter || branch.name.includes(filter) + ); return ( -
- {this._renderItems()} -
+ data[index].name} + itemSize={ITEM_HEIGHT} + style={{ overflowX: 'hidden', paddingTop: 0, paddingBottom: 0 }} + width={'auto'} + > + {this._renderItem} + ); } - /** - * Renders menu items. - * - * @returns array of React elements - */ - private _renderItems(): React.ReactElement[] { - return this.state.branches.map(this._renderItem, this); - } - /** * Renders a menu item. * - * @param branch - branch - * @param idx - item index + * @param props Row properties * @returns React element */ - private _renderItem( - branch: Git.IBranch, - idx: number - ): React.ReactElement | null { - // Perform a "simple" filter... (TODO: consider implementing fuzzy filtering) - if (this.state.filter && !branch.name.includes(this.state.filter)) { - return null; - } + private _renderItem = (props: ListChildComponentProps): JSX.Element => { + const { data, index, style } = props; + const branch = data[index] as Git.IBranch; const isActive = branch.name === this.state.current; return ( ); - } + }; /** * Renders a dialog for creating a new branch. diff --git a/src/components/FileItem.tsx b/src/components/FileItem.tsx index 13188f8c5..be66d9c5f 100644 --- a/src/components/FileItem.tsx +++ b/src/components/FileItem.tsx @@ -25,29 +25,110 @@ export const STATUS_CODES = { '!': 'Ignored' }; +/** + * File marker properties + */ +interface IGitMarkBoxProps { + /** + * Filename + */ + fname: string; + /** + * Git repository model + */ + model: GitExtension; + /** + * File status + */ + stage: Git.Status; +} + +/** + * Render the selection box in simple mode + */ +class GitMarkBox extends React.PureComponent { + protected _onClick = (): void => { + // toggle will emit a markChanged signal + this.props.model.toggleMark(this.props.fname); + + // needed if markChanged doesn't force an update of a parent + this.forceUpdate(); + }; + + protected _onDoubleClick = ( + event: React.MouseEvent + ): void => { + event.stopPropagation(); + }; + + render(): JSX.Element { + // idempotent, will only run once per file + this.props.model.addMark( + this.props.fname, + this.props.stage !== 'untracked' + ); + + return ( + + ); + } +} + +/** + * File item properties + */ export interface IFileItemProps { + /** + * Action buttons on the file + */ actions?: React.ReactElement; + /** + * Callback to open a context menu on the file + */ contextMenu?: (file: Git.IStatusFile, event: React.MouseEvent) => void; + /** + * File model + */ file: Git.IStatusFile; + /** + * Is the file marked? + */ markBox?: boolean; + /** + * Git repository model + */ model: GitExtension; + /** + * Callback on double click + */ onDoubleClick: () => void; + /** + * Is the file selected? + */ selected?: boolean; + /** + * Callback to select the file + */ selectFile?: (file: Git.IStatusFile | null) => void; + /** + * Inline styling for the windowing + */ + style: React.CSSProperties; } -export interface IGitMarkBoxProps { - fname: string; - model: GitExtension; - stage: string; -} - -export class FileItem extends React.Component { - getFileChangedLabel(change: keyof typeof STATUS_CODES): string { +export class FileItem extends React.PureComponent { + protected _getFileChangedLabel(change: keyof typeof STATUS_CODES): string { return STATUS_CODES[change]; } - getFileChangedLabelClass(change: string) { + protected _getFileChangedLabelClass(change: string): string { if (change === 'M') { return this.props.selected ? classes( @@ -67,20 +148,20 @@ export class FileItem extends React.Component { } } - getFileClass() { + protected _getFileClass(): string { return this.props.selected ? classes(fileStyle, selectedFileStyle) : fileStyle; } - render() { + render(): JSX.Element { const { file } = this.props; const status_code = file.status === 'staged' ? file.x : file.y; - const status = this.getFileChangedLabel(status_code as any); + const status = this._getFileChangedLabel(status_code as any); return (
  • this.props.selectFile(this.props.file)) @@ -92,6 +173,7 @@ export class FileItem extends React.Component { }) } onDoubleClick={this.props.onDoubleClick} + style={this.props.style} title={`${this.props.file.to} ● ${status}`} > {this.props.markBox && ( @@ -106,47 +188,10 @@ export class FileItem extends React.Component { selected={this.props.selected} /> {this.props.actions} - + {this.props.file.y === '?' ? 'U' : status_code}
    • ); } } - -export class GitMarkBox extends React.Component { - constructor(props: IGitMarkBoxProps) { - super(props); - } - - protected _onClick = (event: React.ChangeEvent) => { - // toggle will emit a markChanged signal - this.props.model.toggleMark(this.props.fname); - - // needed if markChanged doesn't force an update of a parent - this.forceUpdate(); - }; - - protected _onDoubleClick = (event: React.MouseEvent) => { - event.stopPropagation(); - }; - - render() { - // idempotent, will only run once per file - this.props.model.addMark( - this.props.fname, - this.props.stage !== 'untracked' - ); - - return ( - - ); - } -} diff --git a/src/components/FileList.tsx b/src/components/FileList.tsx index 84c275863..549a90a51 100644 --- a/src/components/FileList.tsx +++ b/src/components/FileList.tsx @@ -3,6 +3,8 @@ import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { CommandRegistry } from '@lumino/commands'; import { Menu } from '@lumino/widgets'; import * as React from 'react'; +import AutoSizer from 'react-virtualized-auto-sizer'; +import { ListChildComponentProps } from 'react-window'; import { CommandIDs } from '../commandsAndMenu'; import { GitExtension } from '../model'; import { hiddenButtonStyle } from '../style/ActionButtonStyle'; @@ -223,11 +225,16 @@ export class FileList extends React.Component { return this.props.files.filter(file => this.props.model.getMark(file.to)); } + /** + * Render the modified files + */ render() { if (this.props.settings.composite['simpleStaging']) { return (
    - {this._renderSimpleStage(this.props.files)} + + {({ height }) => this._renderSimpleStage(this.props.files, height)} +
    ); } else { @@ -267,14 +274,24 @@ export class FileList extends React.Component { className={fileListWrapperClass} onContextMenu={event => event.preventDefault()} > - {this._renderStaged(stagedFiles)} - {this._renderChanged(unstagedFiles)} - {this._renderUntracked(untrackedFiles)} + + {({ height }) => ( + <> + {this._renderStaged(stagedFiles, height)} + {this._renderChanged(unstagedFiles, height)} + {this._renderUntracked(untrackedFiles, height)} + + )} + ); } } + /** + * Test if a file is selected + * @param candidate file to test + */ private _isSelectedFile(candidate: Git.IStatusFile): boolean { if (this.state.selectedFile === null) { return false; @@ -289,9 +306,70 @@ export class FileList extends React.Component { ); } - private _renderStaged(files: Git.IStatusFile[]) { + /** + * Render a staged file + * + * Note: This is actually a React.FunctionComponent but defined as + * a private method as it needs access to FileList properties. + * + * @param rowProps Row properties + */ + private _renderStagedRow = ( + rowProps: ListChildComponentProps + ): JSX.Element => { const doubleClickDiff = this.props.settings.get('doubleClickDiff') .composite as boolean; + const { data, index, style } = rowProps; + const file = data[index] as Git.IStatusFile; + const openFile = () => { + this.props.commands.execute(CommandIDs.gitFileOpen, file as any); + }; + const diffButton = this._createDiffButton(file); + return ( + + + {diffButton} + { + this.resetStagedFile(file.to); + }} + /> + + } + file={file} + contextMenu={this.openContextMenu} + model={this.props.model} + selected={this._isSelectedFile(file)} + selectFile={this.updateSelectedFile} + onDoubleClick={ + doubleClickDiff + ? diffButton + ? () => this._openDiffView(file) + : () => undefined + : openFile + } + style={style} + /> + ); + }; + + /** + * Render the staged files list. + * + * @param files The staged files + * @param height The height of the HTML element + */ + private _renderStaged(files: Git.IStatusFile[], height: number) { return ( { /> } collapsible + files={files} heading={'Staged'} - nFiles={files.length} - > - {files.map((file: Git.IStatusFile) => { - const openFile = () => { - this.props.commands.execute(CommandIDs.gitFileOpen, file as any); - }; - const diffButton = this._createDiffButton(file); - return ( - - - {diffButton} - { - this.resetStagedFile(file.to); - }} - /> - - } - file={file} - contextMenu={this.openContextMenu} - model={this.props.model} - selected={this._isSelectedFile(file)} - selectFile={this.updateSelectedFile} - onDoubleClick={ - doubleClickDiff - ? diffButton - ? () => this._openDiffView(file) - : () => undefined - : openFile - } - /> - ); - })} - + height={height} + rowRenderer={this._renderStagedRow} + /> ); } - private _renderChanged(files: Git.IStatusFile[]) { + /** + * Render a changed file + * + * Note: This is actually a React.FunctionComponent but defined as + * a private method as it needs access to FileList properties. + * + * @param rowProps Row properties + */ + private _renderChangedRow = ( + rowProps: ListChildComponentProps + ): JSX.Element => { const doubleClickDiff = this.props.settings.get('doubleClickDiff') .composite as boolean; + const { data, index, style } = rowProps; + const file = data[index] as Git.IStatusFile; + const openFile = () => { + this.props.commands.execute(CommandIDs.gitFileOpen, file as any); + }; + const diffButton = this._createDiffButton(file); + return ( + + + {diffButton} + { + this.discardChanges(file); + }} + /> + { + this.addFile(file.to); + }} + /> + + } + file={file} + contextMenu={this.openContextMenu} + model={this.props.model} + selected={this._isSelectedFile(file)} + selectFile={this.updateSelectedFile} + onDoubleClick={ + doubleClickDiff + ? diffButton + ? () => this._openDiffView(file) + : () => undefined + : openFile + } + style={style} + /> + ); + }; + + /** + * Render the changed files list + * + * @param files Changed files + * @param height Height of the HTML element + */ + private _renderChanged(files: Git.IStatusFile[], height: number) { const disabled = files.length === 0; return ( { } collapsible heading={'Changed'} - nFiles={files.length} - > - {files.map((file: Git.IStatusFile) => { - const openFile = () => { - this.props.commands.execute(CommandIDs.gitFileOpen, file as any); - }; - const diffButton = this._createDiffButton(file); - return ( - - - {diffButton} - { - this.discardChanges(file); - }} - /> - { - this.addFile(file.to); - }} - /> - - } - file={file} - contextMenu={this.openContextMenu} - model={this.props.model} - selected={this._isSelectedFile(file)} - selectFile={this.updateSelectedFile} - onDoubleClick={ - doubleClickDiff - ? diffButton - ? () => this._openDiffView(file) - : () => undefined - : openFile - } - /> - ); - })} - + height={height} + files={files} + rowRenderer={this._renderChangedRow} + /> ); } - private _renderUntracked(files: Git.IStatusFile[]) { + /** + * Render a untracked file. + * + * Note: This is actually a React.FunctionComponent but defined as + * a private method as it needs access to FileList properties. + * + * @param rowProps Row properties + */ + private _renderUntrackedRow = ( + rowProps: ListChildComponentProps + ): JSX.Element => { const doubleClickDiff = this.props.settings.get('doubleClickDiff') .composite as boolean; + const { data, index, style } = rowProps; + const file = data[index] as Git.IStatusFile; + return ( + + { + this.props.commands.execute( + CommandIDs.gitFileOpen, + file as any + ); + }} + /> + { + this.addFile(file.to); + }} + /> + + } + file={file} + contextMenu={this.openContextMenu} + model={this.props.model} + onDoubleClick={() => { + if (!doubleClickDiff) { + this.props.commands.execute(CommandIDs.gitFileOpen, file as any); + } + }} + selected={this._isSelectedFile(file)} + selectFile={this.updateSelectedFile} + style={style} + /> + ); + }; + + /** + * Render the untracked files list. + * + * @param files Untracked files + * @param height Height of the HTML element + */ + private _renderUntracked(files: Git.IStatusFile[], height: number) { return ( { } collapsible heading={'Untracked'} - nFiles={files.length} - > - {files.map((file: Git.IStatusFile) => { - return ( - - { - this.props.commands.execute( - CommandIDs.gitFileOpen, - file as any - ); - }} - /> - { - this.addFile(file.to); - }} - /> - - } - file={file} - contextMenu={this.openContextMenu} - model={this.props.model} - onDoubleClick={() => { - if (!doubleClickDiff) { - this.props.commands.execute( - CommandIDs.gitFileOpen, - file as any - ); - } - }} - selected={this._isSelectedFile(file)} - selectFile={this.updateSelectedFile} - /> - ); - })} - + height={height} + files={files} + rowRenderer={this._renderUntrackedRow} + /> ); } - private _renderSimpleStage(files: Git.IStatusFile[]) { + /** + * Render a modified file in simple mode. + * + * Note: This is actually a React.FunctionComponent but defined as + * a private method as it needs access to FileList properties. + * + * @param rowProps Row properties + */ + private _renderSimpleStageRow = (rowProps: ListChildComponentProps) => { + const { data, index, style } = rowProps; + const file = data[index] as Git.IStatusFile; const doubleClickDiff = this.props.settings.get('doubleClickDiff') .composite as boolean; + + const openFile = () => { + this.props.commands.execute(CommandIDs.gitFileOpen, file as any); + }; + + // Default value for actions and double click + let actions: JSX.Element = ( + + ); + let onDoubleClick = doubleClickDiff ? (): void => undefined : openFile; + + if (file.status === 'unstaged' || file.status === 'partially-staged') { + const diffButton = this._createDiffButton(file); + actions = ( + + + {diffButton} + { + this.discardChanges(file); + }} + /> + + ); + onDoubleClick = doubleClickDiff + ? diffButton + ? () => this._openDiffView(file) + : () => undefined + : openFile; + } else if (file.status === 'staged') { + const diffButton = this._createDiffButton(file); + actions = ( + + + {diffButton} + { + this.discardChanges(file); + }} + /> + + ); + onDoubleClick = doubleClickDiff + ? diffButton + ? () => this._openDiffView(file) + : () => undefined + : openFile; + } + + return ( + + ); + }; + + /** + * Render the modified files in simple mode. + * + * @param files Modified files + * @param height Height of the HTML element + */ + private _renderSimpleStage(files: Git.IStatusFile[], height: number) { return ( { /> } heading={'Changed'} - nFiles={files.length} - > - {files.map((file: Git.IStatusFile) => { - const openFile = () => { - this.props.commands.execute(CommandIDs.gitFileOpen, file as any); - }; - - // Default value for actions and double click - let actions: JSX.Element = ( - - ); - let onDoubleClick = doubleClickDiff - ? (): void => undefined - : openFile; - - if ( - file.status === 'unstaged' || - file.status === 'partially-staged' - ) { - const diffButton = this._createDiffButton(file); - actions = ( - - - {diffButton} - { - this.discardChanges(file); - }} - /> - - ); - onDoubleClick = doubleClickDiff - ? diffButton - ? () => this._openDiffView(file) - : () => undefined - : openFile; - } else if (file.status === 'staged') { - const diffButton = this._createDiffButton(file); - actions = ( - - - {diffButton} - { - this.discardChanges(file); - }} - /> - - ); - onDoubleClick = doubleClickDiff - ? diffButton - ? () => this._openDiffView(file) - : () => undefined - : openFile; - } - - return ( - - ); - })} - + height={height} + files={files} + rowRenderer={this._renderSimpleStageRow} + /> ); } diff --git a/src/components/GitStage.tsx b/src/components/GitStage.tsx index 2daaff627..db5c055af 100644 --- a/src/components/GitStage.tsx +++ b/src/components/GitStage.tsx @@ -1,5 +1,6 @@ import { caretDownIcon, caretRightIcon } from '@jupyterlab/ui-components'; import * as React from 'react'; +import { FixedSizeList, ListChildComponentProps } from 'react-window'; import { changeStageButtonStyle, sectionAreaStyle, @@ -7,6 +8,10 @@ import { sectionHeaderLabelStyle, sectionHeaderSizeStyle } from '../style/GitStageStyle'; +import { Git } from '../tokens'; + +const HEADER_HEIGHT = 34; +const ITEM_HEIGHT = 25; /** * Git stage component properties @@ -20,14 +25,22 @@ export interface IGitStageProps { * Is this group collapsible */ collapsible?: boolean; + /** + * Files in the group + */ + files: Git.IStatusFile[]; /** * Group title */ heading: string; /** - * Number of files in the group + * HTML element height + */ + height: number; + /** + * Row renderer */ - nFiles: number; + rowRenderer: (props: ListChildComponentProps) => JSX.Element; } /** @@ -38,23 +51,24 @@ export interface IGitStageState { } export const GitStage: React.FunctionComponent = ( - props: React.PropsWithChildren + props: IGitStageProps ) => { const [showFiles, setShowFiles] = React.useState(true); + const nFiles = props.files.length; return (
    -
    +
    { + if (props.collapsible && nFiles > 0) { + setShowFiles(!showFiles); + } + }} + > {props.collapsible && ( -
    - {showFiles && ( -
      {props.children}
    + {showFiles && nFiles > 0 && ( + data[index].to} + itemSize={ITEM_HEIGHT} + style={{ overflowX: 'hidden' }} + width={'auto'} + > + {props.rowRenderer} + )}
    ); diff --git a/src/components/NewBranchDialog.tsx b/src/components/NewBranchDialog.tsx index 81bda71e3..cbe4a5801 100644 --- a/src/components/NewBranchDialog.tsx +++ b/src/components/NewBranchDialog.tsx @@ -1,6 +1,5 @@ import * as React from 'react'; import { classes } from 'typestyle'; -import List from '@material-ui/core/List'; import ListItem from '@material-ui/core/ListItem'; import Dialog from '@material-ui/core/Dialog'; import DialogActions from '@material-ui/core/DialogActions'; @@ -34,6 +33,7 @@ import { } from '../style/NewBranchDialog'; import { SuspendModal } from './SuspendModal'; import { Alert } from './Alert'; +import { VariableSizeList, ListChildComponentProps } from 'react-window'; const BRANCH_DESC = { current: @@ -42,6 +42,10 @@ const BRANCH_DESC = { 'The default branch. Pick this if you want to start fresh from the default branch.' }; +const ITEM_HEIGHT = 27.5; // HTML element height for a single branch +const CURRENT_BRANCH_HEIGHT = 66.5; // HTML element height for the current branch with description +const HEIGHT = 200; // HTML element height for the branches list + /** * Interface describing component properties. */ @@ -134,6 +138,7 @@ export class NewBranchDialog extends React.Component< super(props); const repo = this.props.model.pathRepository; + this._branchList = React.createRef(); this.state = { name: '', @@ -238,9 +243,7 @@ export class NewBranchDialog extends React.Component< ) : null}
    -
    - {this._renderItems()} -
    + {this._renderItems()} !filter || branch.name.includes(filter)) .slice() - .sort(comparator) - .map(this._renderItem, this); + .sort(comparator); + return ( + data[index].name} + itemSize={index => { + const branch = branches[index]; + return branch.name === this.state.current + ? CURRENT_BRANCH_HEIGHT + : ITEM_HEIGHT; + }} + ref={this._branchList} + style={{ overflowX: 'hidden' }} + width={'auto'} + > + {this._renderItem} + + ); /** * Comparator function for sorting branches. @@ -301,18 +327,13 @@ export class NewBranchDialog extends React.Component< /** * Renders a branch menu item. * - * @param branch - branch - * @param idx - item index + * @param props Row properties * @returns React element */ - private _renderItem( - branch: Git.IBranch, - idx: number - ): React.ReactElement | null { - // Perform a "simple" filter... (TODO: consider implementing fuzzy filtering) - if (this.state.filter && !branch.name.includes(this.state.filter)) { - return null; - } + private _renderItem = (props: ListChildComponentProps): JSX.Element => { + const { data, index, style } = props; + const branch = data[index] as Git.IBranch; + const isBase = branch.name === this.state.base; const isCurr = branch.name === this.state.current; @@ -327,8 +348,8 @@ export class NewBranchDialog extends React.Component< button title={`Create a new branch based on: ${branch.name}`} className={classes(listItemClass, isBase ? activeListItemClass : null)} - key={branch.name} onClick={this._onBranchClickFactory(branch.name)} + style={style} >     ); - } + }; /** * Renders a component to provide UI feedback. @@ -451,6 +472,7 @@ export class NewBranchDialog extends React.Component< * @param event - event object */ private _onFilterChange = (event: any): void => { + this._branchList.current.resetAfterIndex(0); this.setState({ filter: event.target.value }); @@ -460,6 +482,7 @@ export class NewBranchDialog extends React.Component< * Callback invoked to reset the menu filter. */ private _resetFilter = (): void => { + this._branchList.current.resetAfterIndex(0); this.setState({ filter: '' }); @@ -564,6 +587,7 @@ export class NewBranchDialog extends React.Component< this.props.onClose(); // Reset the branch name and filter: + this._branchList.current.resetAfterIndex(0); this.setState({ name: '', filter: '' @@ -589,4 +613,6 @@ export class NewBranchDialog extends React.Component< alert: false }); }; + + private _branchList: React.RefObject; } diff --git a/src/components/SinglePastCommitInfo.tsx b/src/components/SinglePastCommitInfo.tsx index 98b57cfea..5902c9771 100644 --- a/src/components/SinglePastCommitInfo.tsx +++ b/src/components/SinglePastCommitInfo.tsx @@ -1,6 +1,7 @@ import { fileIcon } from '@jupyterlab/ui-components'; import { CommandRegistry } from '@lumino/commands'; import * as React from 'react'; +import { FixedSizeList, ListChildComponentProps } from 'react-window'; import { classes } from 'typestyle/'; import { CommandIDs } from '../commandsAndMenu'; import { GitExtension } from '../model'; @@ -29,6 +30,9 @@ import { isDiffSupported } from './diff/Diff'; import { FilePath } from './FilePath'; import { ResetRevertDialog } from './ResetRevertDialog'; +const ITEM_HEIGHT = 24; // File list item height +const MAX_VISIBLE_FILES = 20; // Maximal number of file display at once + /** * Interface describing component properties. */ @@ -130,18 +134,9 @@ export class SinglePastCommitInfo extends React.Component< * Callback invoked immediately after mounting a component (i.e., inserting into a tree). */ async componentDidMount(): Promise { - let log: Git.ISingleCommitFilePathInfo; try { - log = await this.props.model.detailedLog(this.props.commit.commit); - } catch (err) { - console.error( - `Error while getting detailed log for commit ${this.props.commit.commit} and path ${this.props.model.pathRepository}`, - err - ); - this.setState({ loadingState: 'error' }); - return; - } - if (log.code === 0) { + const log = await this.props.model.detailedLog(this.props.commit.commit); + this.setState({ info: log.modified_file_note, numFiles: log.modified_files_count, @@ -150,6 +145,13 @@ export class SinglePastCommitInfo extends React.Component< modifiedFiles: log.modified_files, loadingState: 'success' }); + } catch (err) { + console.error( + `Error while getting detailed log for commit ${this.props.commit.commit} and path ${this.props.model.pathRepository}`, + err + ); + this.setState({ loadingState: 'error' }); + return; } } @@ -213,40 +215,45 @@ export class SinglePastCommitInfo extends React.Component< onClose={this._onResetRevertDialogClose} /> -
      - {this.state.modifiedFiles.length > 0 - ? this._renderFileList() - : null} -
    + {this.state.modifiedFiles.length > 0 && ( + data[index].modified_file_path} + itemSize={ITEM_HEIGHT} + style={{ overflowX: 'hidden' }} + width={'auto'} + > + {this._renderFile} + + )} ); } - /** - * Renders a list of modified files. - * - * @returns array of React elements - */ - private _renderFileList(): React.ReactElement[] { - return this.state.modifiedFiles.map(this._renderFile, this); - } - /** * Renders a modified file. * - * @param file - modified file - * @param idx - file index + * @param props Row properties * @returns React element */ - private _renderFile(file: Git.ICommitModifiedFile): React.ReactElement { + private _renderFile = (props: ListChildComponentProps): JSX.Element => { + const { data, index, style } = props; + const file = data[index]; const path = file.modified_file_path; const flg = isDiffSupported(path) || !file.is_binary; return (
  • @@ -255,7 +262,7 @@ export class SinglePastCommitInfo extends React.Component< ) : null}
    • ); - } + }; /** * Callback invoked upon a clicking a button to revert changes. diff --git a/src/style/BranchMenu.ts b/src/style/BranchMenu.ts index 117ef6e4e..cd5064b02 100644 --- a/src/style/BranchMenu.ts +++ b/src/style/BranchMenu.ts @@ -94,16 +94,6 @@ export const newBranchButtonClass = style({ borderRadius: '3px' }); -export const listWrapperClass = style({ - display: 'block', - width: '100%', - minHeight: '150px', - maxHeight: '400px', - - overflow: 'hidden', - overflowY: 'auto' -}); - export const listItemClass = style({ paddingTop: '4px!important', paddingBottom: '4px!important', diff --git a/src/style/FileItemStyle.ts b/src/style/FileItemStyle.ts index 38c05f058..ecd697e95 100644 --- a/src/style/FileItemStyle.ts +++ b/src/style/FileItemStyle.ts @@ -6,6 +6,7 @@ export const fileStyle = style( display: 'flex', flexDirection: 'row', alignItems: 'center', + boxSizing: 'border-box', color: 'var(--jp-ui-font-color1)', lineHeight: 'var(--jp-private-running-item-height)', padding: '0px 4px', diff --git a/src/style/FileListStyle.ts b/src/style/FileListStyle.ts index 5155617ac..e80df39b8 100644 --- a/src/style/FileListStyle.ts +++ b/src/style/FileListStyle.ts @@ -1,7 +1,7 @@ import { style } from 'typestyle'; export const fileListWrapperClass = style({ - height: 'auto', + height: 'inherit', minHeight: '150px', overflow: 'hidden', diff --git a/src/style/NewBranchDialog.ts b/src/style/NewBranchDialog.ts index bbd13c6b5..450908e58 100644 --- a/src/style/NewBranchDialog.ts +++ b/src/style/NewBranchDialog.ts @@ -171,15 +171,10 @@ export const filterClearClass = style({ export const listWrapperClass = style({ boxSizing: 'border-box', display: 'block', - - width: '100%', - height: '200px', - border: 'var(--jp-border-width) solid var(--jp-border-color2)', borderRadius: '3px', - - overflow: 'hidden', - overflowY: 'scroll' + paddingTop: 0, + paddingBottom: 0 }); export const listItemClass = style({ diff --git a/src/style/PastCommitNode.ts b/src/style/PastCommitNode.ts index 39882dac1..cc763e52d 100644 --- a/src/style/PastCommitNode.ts +++ b/src/style/PastCommitNode.ts @@ -5,7 +5,7 @@ export const commitWrapperClass = style({ display: 'flex', flexShrink: 0, flexDirection: 'column', - padding: '10px', + padding: '5px 0px 5px 10px', borderBottom: 'var(--jp-border-width) solid var(--jp-border-color2)' }); diff --git a/tests/test-components/BranchMenu.spec.tsx b/tests/test-components/BranchMenu.spec.tsx index 434380a42..d11876eb4 100644 --- a/tests/test-components/BranchMenu.spec.tsx +++ b/tests/test-components/BranchMenu.spec.tsx @@ -1,4 +1,4 @@ -import { shallow } from 'enzyme'; +import { mount, render, shallow } from 'enzyme'; import 'jest'; import * as React from 'react'; import { BranchMenu } from '../../src/components/BranchMenu'; @@ -192,7 +192,7 @@ describe('BranchMenu', () => { branching: false, suspend: false }; - const component = shallow(); + const component = render(); const nodes = component.find(`.${listItemClass}`); const branches = model.branches; @@ -201,11 +201,8 @@ describe('BranchMenu', () => { // Should contain the branch names... for (let i = 0; i < branches.length; i++) { expect( - nodes - .at(i) - .text() - .includes(branches[i].name) - ).toEqual(true); + nodes[i].lastChild.data + ).toEqual(branches[i].name); } }); @@ -215,14 +212,14 @@ describe('BranchMenu', () => { branching: false, suspend: false }; - const component = shallow(); + const component = render(); const nodes = component.find(`.${listItemClass}`); const branches = model.branches; expect(nodes.length).toEqual(branches.length); for (let i = 0; i < branches.length; i++) { - expect(nodes.at(i).prop('title').length > 0).toEqual(true); + expect(nodes[i].attribs['title'].length).toBeGreaterThan(0); } }); @@ -261,17 +258,15 @@ describe('BranchMenu', () => { branching: false, suspend: false }; - const component = shallow(); - const nodes = component.find(`.${listItemClass}`); - - const node = nodes.at(1); - node.simulate('click'); + const component = mount(); + const nodes = component.find(`.${listItemClass}[title*="${BRANCHES[1].name}"]`); + nodes.at(0).simulate('click'); expect(spy).toHaveBeenCalledTimes(0); spy.mockRestore(); }); - it('should not switch to a specified branch upon clicking its corresponding element when branching is enabled', () => { + it('should switch to a specified branch upon clicking its corresponding element when branching is enabled', () => { const spy = jest.spyOn(GitExtension.prototype, 'checkout'); const props = { @@ -279,11 +274,9 @@ describe('BranchMenu', () => { branching: true, suspend: false }; - const component = shallow(); - const nodes = component.find(`.${listItemClass}`); - - const node = nodes.at(1); - node.simulate('click'); + const component = mount(); + const nodes = component.find(`.${listItemClass}[title*="${BRANCHES[1].name}"]`); + nodes.at(0).simulate('click'); expect(spy).toHaveBeenCalledTimes(1); expect(spy).toHaveBeenCalledWith({ diff --git a/tests/test-components/FileItem.spec.tsx b/tests/test-components/FileItem.spec.tsx index 00d4b75a1..7c4af1cd9 100644 --- a/tests/test-components/FileItem.spec.tsx +++ b/tests/test-components/FileItem.spec.tsx @@ -17,7 +17,8 @@ describe('FileItem', () => { model: null, onDoubleClick: () => {}, selected: false, - selectFile: () => {} + selectFile: () => {}, + style: {} }; describe('#render()', () => { diff --git a/yarn.lock b/yarn.lock index 8a1c722b1..baa93f9c8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1822,6 +1822,20 @@ dependencies: "@types/react" "*" +"@types/react-virtualized-auto-sizer@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.0.tgz#fc32f30a8dab527b5816f3a757e1e1d040c8f272" + integrity sha512-NMErdIdSnm2j/7IqMteRiRvRulpjoELnXWUwdbucYCz84xG9PHcoOrr7QfXwB/ku7wd6egiKFrzt/+QK4Imeeg== + dependencies: + "@types/react" "*" + +"@types/react-window@^1.8.2": + version "1.8.2" + resolved "https://registry.yarnpkg.com/@types/react-window/-/react-window-1.8.2.tgz#a5a6b2762ce73ffaab7911ee1397cf645f2459fe" + integrity sha512-gP1xam68Wc4ZTAee++zx6pTdDAH08rAkQrWm4B4F/y6hhmlT9Mgx2q8lTCXnrPHXsr15XjRN9+K2DLKcz44qEQ== + dependencies: + "@types/react" "*" + "@types/react@*", "@types/react@~16.8.13", "@types/react@~16.8.4", "@types/react@~16.9.16": version "16.8.25" resolved "https://registry.yarnpkg.com/@types/react/-/react-16.8.25.tgz#0247613ab58b1b11ba10fed662e1947c5f2bb89c" @@ -5150,6 +5164,11 @@ matcher@^1.0.0: dependencies: escape-string-regexp "^1.0.4" +"memoize-one@>=3.1.1 <6": + version "5.1.1" + resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.1.1.tgz#047b6e3199b508eaec03504de71229b8eb1d75c0" + integrity sha512-HKeeBpWvqiVJD57ZUAsJNm71eHTykffzcLZVYWiVfQeI1rJtuEaS7hQiEpWfVVk18donPwJEcFKIkCmPJNOhHA== + merge-stream@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/merge-stream/-/merge-stream-2.0.0.tgz#52823629a14dd00c9770fb6ad47dc6310f2c1f60" @@ -6040,6 +6059,19 @@ react-transition-group@^4.4.0: loose-envify "^1.4.0" prop-types "^15.6.2" +react-virtualized-auto-sizer@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.2.tgz#a61dd4f756458bbf63bd895a92379f9b70f803bd" + integrity sha512-MYXhTY1BZpdJFjUovvYHVBmkq79szK/k7V3MO+36gJkWGkrXKtyr4vCPtpphaTLRAdDNoYEYFZWE8LjN+PIHNg== + +react-window@^1.8.5: + version "1.8.5" + resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.8.5.tgz#a56b39307e79979721021f5d06a67742ecca52d1" + integrity sha512-HeTwlNa37AFa8MDZFZOKcNEkuF2YflA0hpGPiTT9vR7OawEt+GZbfM6wqkBahD3D3pUjIabQYzsnY/BSJbgq6Q== + dependencies: + "@babel/runtime" "^7.0.0" + memoize-one ">=3.1.1 <6" + react@~16.9.0: version "16.9.0" resolved "https://registry.yarnpkg.com/react/-/react-16.9.0.tgz#40ba2f9af13bc1a38d75dbf2f4359a5185c4f7aa"