Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git diff list uses the perfect scroll bar now. #6085

Merged
merged 1 commit into from
Sep 3, 2019

Conversation

jbicker
Copy link
Contributor

@jbicker jbicker commented Sep 2, 2019

What it does

The ListContainer of the GitDiff View uses the PerfectScrollbar instead of the browser native scrollbar now.
It fixes also an error which caused that sometimes, after resizing vertically, elements at the lower end weren't selectable anymore.
See gitpod-io/gitpod#757

How to test

Use a repository where are enough different files between two branches and use the Git Diff command to open these changes.
The scrollbar of the git diff widget should show the same scrollbar in all browsers.
All elements should be selectable even after resizing.

Review checklist

Reminder for reviewers

@akosyakov akosyakov added the git issues related to git label Sep 3, 2019
@AlexTugarev
Copy link
Contributor

current issues on master:

2019-09-03 08 21 50

vs.

this PR 🎉

2019-09-03 08 22 24

}

focus(): void {
if (this.listContainer) {
this.listContainer.focus();
}
}

getListContainer(): HTMLDivElement | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused

@@ -67,6 +69,10 @@ export class GitDiffWidget extends GitNavigableListWidget<GitFileChangeNode> imp
}));
}

protected getScrollContainer(): MaybePromise<HTMLElement> {
return new Promise(resolve => this.fileChangeListContainerIsReady = resolve);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the Deferred from @theia/core/lib/common/promise-util for such cases

}

componentDidUpdate(): void {
this.props.onListContainerReady(this.listContainer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: does it actually change?

}
})();
}

protected onResize(msg: Widget.ResizeMessage): void {
super.onResize(msg);
this.update();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -413,11 +428,20 @@ export class GitDiffListContainer extends React.Component<GitDiffListContainer.P

componentDidMount(): void {
this.props.addDiffListKeyListeners(this.props.id);
this.props.onListContainerReady(this.listContainer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of renaming onListContainerReady –> setListContainer? onSomething sounds like an event handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@akosyakov
Copy link
Member

please get rid of commits fixing issues in the original commit

@jbicker
Copy link
Contributor Author

jbicker commented Sep 3, 2019

@AlexTugarev and @akosyakov : thanks for reviewing and the hints.

Signed-off-by: Jan Bicker <jan.bicker@typefox.io>
Copy link
Contributor

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked perfect for me!
Thanks!

@jbicker jbicker merged commit 4852766 into master Sep 3, 2019
@jbicker jbicker deleted the git-diff-perfect-scrollbar branch September 3, 2019 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
git issues related to git
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants