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

List some branches #1126

Merged
merged 49 commits into from
Apr 5, 2017
Merged

List some branches #1126

merged 49 commits into from
Apr 5, 2017

Conversation

niik
Copy link
Member

@niik niik commented Apr 4, 2017

image

In order to make this possible I have

  • Extracted the list part of the Branches foldout into its own component - BranchList
  • Deprecated the AutoSizer component from react-virtualized in favor of ResizeObserver
    • I didn't want to do this but the AutoSizer failed to provide a correct size when used inside a dialog. The dialog gets mounted in the DOM but doesn't appear until we call showModal so I assume that has something to do with it. Either way, I couldn't get it to work except by holding off on mounting the list until some arbitrary time after we'd called showModal.
    • This degrades gracefully in case experimentalFeatures is turned off by going back to using the AutoSizer.
    • Upsides with this approach is that we get rid of one more layer of nesting and the hacky way that the AutoSizer detects resizes (it injects a tracking element into the DOM).
  • Move selection tracking up from FilterList into consuming components

Additionally this removes one layer of nesting on Windows by returning an array from List#renderContents instead of wrapping the grid and the fake scroll inside a div.

Fixes #749

niik added 30 commits April 3, 2017 16:25
Obsoleted when expandable foldouts was removed
For some reason this regressed at some point and the only way I could get it to work was by expanding the width some arbitrary amount. At the same time I moved the scrollbar out by 3px so that it wouldn't interfere with the resize handle of a Resizable component
@niik niik changed the title [WIP] List some branches List some branches Apr 4, 2017
@niik
Copy link
Member Author

niik commented Apr 4, 2017

I think this is ready for review!

@joshaber joshaber self-assigned this Apr 5, 2017
Copy link
Contributor

@joshaber joshaber left a comment

Choose a reason for hiding this comment

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

Looks great, just a few notes.

@@ -222,9 +222,31 @@ export interface IRepositoryState {
}

export interface IBranchesState {
/**

This comment was marked as spam.


private getGroupLabel(identifier: BranchGroupIdentifier) {
if (identifier === 'default') {
return 'Default Branch'

This comment was marked as spam.

This comment was marked as spam.

* selected row is clicked on.
*
* @param selectedItem - The item that was just selected
* @param source - The kind of user action that provoced the change,

This comment was marked as spam.

}
}

private onItemClick = (item: Branch) => {

This comment was marked as spam.

This comment was marked as spam.

@niik
Copy link
Member Author

niik commented Apr 5, 2017

🐨

Copy link
Contributor

@joshaber joshaber left a comment

Choose a reason for hiding this comment

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

👁‍🗨

const branch = item.branch
this.props.onItemClick(branch)
if (this.props.onItemClick) {
this.props.onItemClick(item.branch)

This comment was marked as spam.

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants