From f2f0b45f4da5de10fc4c9c724724f252ed3e0561 Mon Sep 17 00:00:00 2001 From: Frederic Collonval Date: Tue, 29 Sep 2020 21:39:43 +0200 Subject: [PATCH 1/3] Display tag similarly to branches --- src/components/TagMenu.tsx | 309 +++++++++++++++++++++++++++++++++++++ src/components/Toolbar.tsx | 146 +++++++++++------- src/widgets/TagList.ts | 124 --------------- 3 files changed, 395 insertions(+), 184 deletions(-) create mode 100644 src/components/TagMenu.tsx delete mode 100644 src/widgets/TagList.ts diff --git a/src/components/TagMenu.tsx b/src/components/TagMenu.tsx new file mode 100644 index 000000000..9b3d7914b --- /dev/null +++ b/src/components/TagMenu.tsx @@ -0,0 +1,309 @@ +import { Dialog, showDialog, showErrorMessage } from '@jupyterlab/apputils'; +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 { Logger } from '../logger'; +import { + filterClass, + filterClearClass, + filterInputClass, + filterWrapperClass, + listItemClass, + listItemIconClass, + wrapperClass +} from '../style/BranchMenu'; +import { tagIcon } from '../style/icons'; +import { IGitExtension, Level } from '../tokens'; + +const CHANGES_ERR_MSG = + 'The repository contains files with uncommitted changes. Please commit or discard these changes before switching to a tag.'; +const ITEM_HEIGHT = 24.8; // HTML element height for a single branch +const MIN_HEIGHT = 150; // Minimal HTML element height for the tags list +const MAX_HEIGHT = 400; // Maximal HTML element height for the tags list + +/** + * Callback invoked upon encountering an error when switching tags. + * + * @private + * @param error - error + * @param logger - the logger + */ +function onTagError(error: any, logger: Logger): void { + if (error.message.includes('following files would be overwritten')) { + // Empty log message to hide the executing alert + logger.log({ + message: '', + level: Level.INFO + }); + showDialog({ + title: 'Unable to checkout tag', + body: ( + +

+ Your changes to the following files would be overwritten by + switching: +

+ + {error.message + .split('\n') + .slice(1, -3) + .map(renderFileName)} + + + Please commit, stash, or discard your changes before you checkout + tags. + + + ), + buttons: [Dialog.okButton({ label: 'Dismiss' })] + }); + } else { + logger.log({ + level: Level.ERROR, + message: 'Failed to checkout tag.', + error + }); + } +} + +/** + * Renders a file name. + * + * @private + * @param filename - file name + * @returns React element + */ +function renderFileName(filename: string): React.ReactElement { + return {filename}; +} + +/** + * Interface describing component properties. + */ +export interface ITagMenuProps { + /** + * Boolean indicating whether branching is disabled. + */ + branching: boolean; + + /** + * Extension logger + */ + logger: Logger; + + /** + * Git extension data model. + */ + model: IGitExtension; +} + +/** + * Interface describing component state. + */ +export interface ITagMenuState { + /** + * Menu filter. + */ + filter: string; + + /** + * Current list of tags. + */ + tags: string[]; +} + +/** + * React component for rendering a branch menu. + */ +export class TagMenu extends React.Component { + /** + * Returns a React component for rendering a branch menu. + * + * @param props - component properties + * @returns React component + */ + constructor(props: ITagMenuProps) { + super(props); + + this.state = { + filter: '', + tags: [] + }; + } + + componentDidMount() { + this.props.model + .tags() + .then(response => { + this.setState({ + tags: response.tags + }); + }) + .catch(error => { + console.error(error); + this.setState({ + tags: [] + }); + showErrorMessage('Fail to get the tags.', error); + }); + } + + /** + * Renders the component. + * + * @returns React element + */ + render(): React.ReactElement { + return ( +
+ {this._renderFilter()} + {this._renderTagList()} +
+ ); + } + + /** + * Renders a branch input filter. + * + * @returns React element + */ + private _renderFilter(): React.ReactElement { + return ( +
+
+ + {this.state.filter ? ( + + ) : null} +
+
+ ); + } + + /** + * Renders a + * + * @returns React element + */ + private _renderTagList(): React.ReactElement { + // Perform a "simple" filter... (TODO: consider implementing fuzzy filtering) + const filter = this.state.filter; + const tags = this.state.tags.filter(tag => !filter || tag.includes(filter)); + return ( + data[index]} + itemSize={ITEM_HEIGHT} + style={{ overflowX: 'hidden', paddingTop: 0, paddingBottom: 0 }} + width={'auto'} + > + {this._renderItem} + + ); + } + + /** + * Renders a menu item. + * + * @param props Row properties + * @returns React element + */ + private _renderItem = (props: ListChildComponentProps): JSX.Element => { + const { data, index, style } = props; + const tag = data[index] as string; + + return ( + + + {tag} + + ); + }; + + /** + * Callback invoked upon a change to the menu filter. + * + * @param event - event object + */ + private _onFilterChange = (event: any): void => { + this.setState({ + filter: event.target.value + }); + }; + + /** + * Callback invoked to reset the menu filter. + */ + private _resetFilter = (): void => { + this.setState({ + filter: '' + }); + }; + + /** + * Returns a callback which is invoked upon clicking a tag. + * + * @param branch - tag + * @returns callback + */ + private _onTagClickFactory(tag: string) { + const self = this; + return onClick; + + /** + * Callback invoked upon clicking a tag. + * + * @private + * @param event - event object + * @returns promise which resolves upon attempting to switch tags + */ + async function onClick(): Promise { + if (!self.props.branching) { + showErrorMessage('Checkout tags is disabled', CHANGES_ERR_MSG); + return; + } + + self.props.logger.log({ + level: Level.RUNNING, + message: 'Checking tag out...' + }); + + try { + await self.props.model.checkoutTag(tag); + } catch (err) { + return onTagError(err, self.props.logger); + } + + self.props.logger.log({ + level: Level.SUCCESS, + message: 'Tag checkout.' + }); + } + } +} diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx index 5b681ef61..14b4b1bb0 100644 --- a/src/components/Toolbar.tsx +++ b/src/components/Toolbar.tsx @@ -1,4 +1,3 @@ -import { showDialog } from '@jupyterlab/apputils'; import { PathExt } from '@jupyterlab/coreutils'; import { caretDownIcon, @@ -6,17 +5,18 @@ import { refreshIcon } from '@jupyterlab/ui-components'; import { CommandRegistry } from '@lumino/commands'; +import { Tab, Tabs } from '@material-ui/core'; import * as React from 'react'; import { classes } from 'typestyle'; import { CommandIDs } from '../commandsAndMenu'; import { Logger } from '../logger'; import { - branchIcon, - desktopIcon, - pullIcon, - pushIcon, - tagIcon -} from '../style/icons'; + selectedTabClass, + tabClass, + tabIndicatorClass, + tabsClass +} from '../style/GitPanel'; +import { branchIcon, desktopIcon, pullIcon, pushIcon } from '../style/icons'; import { spacer, toolbarButtonClass, @@ -30,10 +30,10 @@ import { toolbarMenuWrapperClass, toolbarNavClass } from '../style/Toolbar'; -import { Git, IGitExtension, Level } from '../tokens'; -import { GitTagDialog } from '../widgets/TagList'; +import { IGitExtension, Level } from '../tokens'; import { ActionButton } from './ActionButton'; import { BranchMenu } from './BranchMenu'; +import { TagMenu } from './TagMenu'; /** * Interface describing component properties. @@ -86,10 +86,20 @@ export interface IToolbarProps { * Interface describing component state. */ export interface IToolbarState { + /** + * Current branch name. + */ + branch: string; + /** * Boolean indicating whether a branch menu is shown. */ branchMenu: boolean; + + /** + * Panel tab identifier. + */ + tab: number; } /** @@ -106,7 +116,8 @@ export class Toolbar extends React.Component { super(props); this.state = { - branchMenu: false + branchMenu: false, + tab: 0 }; } @@ -146,12 +157,6 @@ export class Toolbar extends React.Component { onClick={this._onPushClick} title={'Push committed changes'} /> - { toolbarMenuButtonClass, toolbarMenuButtonEnabledClass )} - title={`Change the current branch: ${this.props.currentBranch}`} + title={'Manage branches and tags'} onClick={this._onBranchClick} > @@ -219,19 +224,74 @@ export class Toolbar extends React.Component { )} - {this.state.branchMenu ? ( - - ) : null} + {this.state.branchMenu ? this._renderTabs() : null} ); } + private _renderTabs(): JSX.Element { + return ( + + { + this.setState({ + tab: tab + }); + }} + > + + + + {this.state.tab === 0 ? this._renderBranches() : this._renderTags()} + + ); + } + + private _renderBranches(): JSX.Element { + return ( + + ); + } + + private _renderTags(): JSX.Element { + return ( + + ); + } + /** * Callback invoked upon clicking a button to pull the latest changes. * @@ -291,38 +351,4 @@ export class Toolbar extends React.Component { }); } }; - - /** - * Callback invoked upon clicking a button to view tags. - * - * @param event - event object - */ - private _onTagClick = async (): Promise => { - const result = await showDialog({ - title: 'Checkout tag', - body: new GitTagDialog(this.props.model) - }); - if (result.button.accept) { - this.props.logger.log({ - level: Level.RUNNING, - message: `Switching to ${result.value}...` - }); - - try { - await this.props.model.checkoutTag(result.value); - - this.props.logger.log({ - level: Level.SUCCESS, - message: `Switched to ${result.value}` - }); - } catch (error) { - console.error(error); - this.props.logger.log({ - level: Level.ERROR, - message: `Fail to checkout tag ${result.value}`, - error - }); - } - } - }; } diff --git a/src/widgets/TagList.ts b/src/widgets/TagList.ts deleted file mode 100644 index 89e25b8f0..000000000 --- a/src/widgets/TagList.ts +++ /dev/null @@ -1,124 +0,0 @@ -import { Spinner } from '@jupyterlab/apputils'; -import { Widget } from '@lumino/widgets'; -import { Git, IGitExtension } from '../tokens'; -/** - * The UI for the content shown within the Git push/pull modal. - */ -export class GitTagDialog extends Widget { - /** - * Instantiates the dialog and makes the relevant service API call. - */ - constructor(model: IGitExtension) { - super(); - this._model = model; - - this._body = this._createBody(); - this.node.appendChild(this._body); - - this._spinner = new Spinner(); - this.node.appendChild(this._spinner.node); - - this._executeGitApi(); - } - - /** - * Call the Git REST API - */ - private _executeGitApi() { - this._model - .tags() - .then(response => { - this._handleResponse(response); - }) - .catch(error => this._handleError(error.message)); - } - - /** - * Handles the response from the server by removing the _spinner and showing the appropriate - * success or error message. - * - * @param response the response from the server API call - */ - private async _handleResponse(response: Git.ITagResult) { - if (response.code === 0) { - this._handleSuccess(response); - } else { - this._handleError(response.message); - } - } - - /** - * Handle failed Git tag REST API call - * - * @param message Error message - */ - private _handleError( - message = 'Unexpected failure. Please check your Jupyter server logs for more details.' - ): void { - this.node.removeChild(this._spinner.node); - this._spinner.dispose(); - - const label = document.createElement('label'); - const text = document.createElement('span'); - text.textContent = 'Tag list fetch failed with error:'; - const errorMessage = document.createElement('span'); - errorMessage.textContent = message; - errorMessage.setAttribute( - 'style', - 'background-color:var(--jp-rendermime-error-background)' - ); - label.appendChild(text); - label.appendChild(document.createElement('p')); - label.appendChild(errorMessage); - this._body.appendChild(label); - } - - /** - * Handle successful Git tag REST API call - * - * @param response Git REST API response - */ - private _handleSuccess(response: Git.ITagResult): void { - this.node.removeChild(this._spinner.node); - this._spinner.dispose(); - - const label = document.createElement('label'); - const text = document.createElement('span'); - text.textContent = 'Select the tag to checkout : '; - // Reverse the tag list to get the more recent on top - const tags = response.tags.reverse(); - this._list = document.createElement('select'); - tags.forEach(tag => { - if (tag) { - const option = document.createElement('option'); - option.value = tag; - option.textContent = tag; - this._list.appendChild(option); - } - }); - label.appendChild(text); - this._body.appendChild(label); - this._body.appendChild(this._list); - } - - /** - * Create the dialog body node - */ - private _createBody(): HTMLElement { - const node = document.createElement('div'); - node.className = 'jp-RedirectForm'; - return node; - } - - /** - * Returns the input value. - */ - getValue(): string { - return this._list.value; - } - - private _body: HTMLElement; - private _list: HTMLSelectElement; - private _model: IGitExtension; - private _spinner: Spinner; -} From c44d2fa8c6289e285350c4467c70169cc872709b Mon Sep 17 00:00:00 2001 From: Frederic Collonval Date: Sun, 4 Oct 2020 16:45:08 +0200 Subject: [PATCH 2/3] Fix styling --- src/style/BranchMenu.ts | 16 +++++++++------- src/style/Toolbar.ts | 13 ++++--------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/style/BranchMenu.ts b/src/style/BranchMenu.ts index e397dc323..566704254 100644 --- a/src/style/BranchMenu.ts +++ b/src/style/BranchMenu.ts @@ -2,22 +2,21 @@ import { style } from 'typestyle'; export const wrapperClass = style({ marginTop: '6px', - marginBottom: '0' + marginBottom: '0', + + borderBottom: 'var(--jp-border-width) solid var(--jp-border-color2)' }); export const filterWrapperClass = style({ - padding: '4px 11px 4px' + padding: '4px 11px 4px', + display: 'flex' }); export const filterClass = style({ + flex: '1 1 auto', boxSizing: 'border-box', display: 'inline-block', position: 'relative', - - width: 'calc(100% - 7.7em - 11px)', // full_width - button_width - right_margin - - marginRight: '11px', - fontSize: 'var(--jp-ui-font-size1)' }); @@ -85,6 +84,9 @@ export const newBranchButtonClass = style({ width: '7.7em', height: '2em', + flex: '0 0 auto', + + marginLeft: '5px', color: 'white', fontSize: 'var(--jp-ui-font-size1)', diff --git a/src/style/Toolbar.ts b/src/style/Toolbar.ts index 2a6c4edad..93120d736 100644 --- a/src/style/Toolbar.ts +++ b/src/style/Toolbar.ts @@ -17,17 +17,11 @@ export const toolbarNavClass = style({ backgroundColor: 'var(--jp-layout-color1)', - borderBottomStyle: 'solid', - borderBottomWidth: 'var(--jp-border-width)', - borderBottomColor: 'var(--jp-border-color2)' + borderBottom: 'var(--jp-border-width) solid var(--jp-border-color2)' }); export const toolbarMenuWrapperClass = style({ - background: 'var(--jp-layout-color1)', - - borderBottomStyle: 'solid', - borderBottomWidth: 'var(--jp-border-width)', - borderBottomColor: 'var(--jp-border-color2)' + background: 'var(--jp-layout-color1)' }); export const toolbarMenuButtonClass = style({ @@ -47,7 +41,8 @@ export const toolbarMenuButtonClass = style({ color: 'var(--jp-ui-font-color1)', textAlign: 'left', - border: 'none', + border: 'none', + borderBottom: 'var(--jp-border-width) solid var(--jp-border-color2)', borderRadius: 0, background: 'var(--jp-layout-color1)' From bef657394de96a69ea6916989d4eb59259043e7c Mon Sep 17 00:00:00 2001 From: Frederic Collonval Date: Sun, 4 Oct 2020 16:46:32 +0200 Subject: [PATCH 3/3] Fix unit test --- src/components/Toolbar.tsx | 7 +------ src/style/Toolbar.ts | 2 +- tests/test-components/Toolbar.spec.tsx | 4 +--- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx index 14b4b1bb0..2d0290b6e 100644 --- a/src/components/Toolbar.tsx +++ b/src/components/Toolbar.tsx @@ -30,7 +30,7 @@ import { toolbarMenuWrapperClass, toolbarNavClass } from '../style/Toolbar'; -import { IGitExtension, Level } from '../tokens'; +import { Git, IGitExtension, Level } from '../tokens'; import { ActionButton } from './ActionButton'; import { BranchMenu } from './BranchMenu'; import { TagMenu } from './TagMenu'; @@ -86,11 +86,6 @@ export interface IToolbarProps { * Interface describing component state. */ export interface IToolbarState { - /** - * Current branch name. - */ - branch: string; - /** * Boolean indicating whether a branch menu is shown. */ diff --git a/src/style/Toolbar.ts b/src/style/Toolbar.ts index 93120d736..f41c6af00 100644 --- a/src/style/Toolbar.ts +++ b/src/style/Toolbar.ts @@ -41,7 +41,7 @@ export const toolbarMenuButtonClass = style({ color: 'var(--jp-ui-font-color1)', textAlign: 'left', - border: 'none', + border: 'none', borderBottom: 'var(--jp-border-width) solid var(--jp-border-color2)', borderRadius: 0, diff --git a/tests/test-components/Toolbar.spec.tsx b/tests/test-components/Toolbar.spec.tsx index 57d2e5d00..31dec2f34 100644 --- a/tests/test-components/Toolbar.spec.tsx +++ b/tests/test-components/Toolbar.spec.tsx @@ -278,9 +278,7 @@ describe('Toolbar', () => { const node = shallow(); const button = node.find(`.${toolbarMenuButtonClass}`).at(1); - expect(button.prop('title')).toEqual( - `Change the current branch: ${currentBranch}` - ); + expect(button.prop('title')).toEqual('Manage branches and tags'); }); });