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

Tree list #1254

Merged
merged 3 commits into from
Dec 8, 2020
Merged

Tree list #1254

merged 3 commits into from
Dec 8, 2020

Conversation

jameskerr
Copy link
Member

Here is a new package called tree-list.

This component features the following:

  • Virualized item rendering for long lists
  • Drag and drop re-ordering
  • Multi-selection

Example usage:

<TreeList
  root={root}
  itemHeight={24}
  onItemMove={onItemMove}
  onItemClick={onItemClick}
  onItemContextMenu={onItemContextMenu}
>
  {Item}
</TreeList>

The props are typed like so:

export type TreeListProps = {
  root: {items: TreeItem[]}
  itemHeight: number
  onItemClick: (e: MouseEvent, item: TreeItem) => void
  onItemContextMenu: (e: MouseEvent, item: TreeItem, selections: TreeItem[]) => void
  onItemMove: (source: TreeItem, dest: TreeItem) => void
  children: ItemRenderer
}

export interface TreeItem {
  id: string
}

The single child of a TreeList is the item renderer component. It has a type like so:

type ItemRenderer = (props: ItemRendererProps) => JSX.Element

type ItemRendererProps = {
  item: TreeItem
  itemProps: object
  isSelected: boolean
  innerRef: Ref<Element>
}

I've added a QueriesSection component in this PR that uses the TreeList component. However, merging this into master will not change any behavior of the app. The layout state needs to be migrated to include a new section with {id: "queries"}. @mason, you can cherry-pick this commit to get the migration: fe3a9cb

The SideBar/Item component that I've added is just a placeholder. You can take over from there as you see fit.

Copy link
Contributor

@mason-fish mason-fish left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for all the tests too! I'll pick it up from here 👍

@@ -271,3 +283,41 @@ function HistorySection({isOpen, style, resizeProps, toggleProps}) {
</StyledSection>
)
}

function QueriesSection({isOpen, style, resizeProps, toggleProps}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I'll take over on this and move it out into it's own file like the rest of the sections

import {createContext, useContext} from "react"
import {TreeController} from "../types"

export const TreeListContext = createContext<TreeController>(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh fun, I was hoping we'd start to play with the context api. Have you given much thought to how we want to use it vs. redux vs. state?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I usually pass down props through to the children, but the API for react-window in combination with react-beautiful-dnd didn't allow passing props to the function child. It's a bug (atlassian/react-beautiful-dnd#1647 (comment)). So in order to get props from the parent (TreeList) to the child (TreeItem), I had to use context.

Redux I think should be for stuff that needs to be shared across many components and/or persisted. Local state + props should be used for anything else. This context thing was really just a work around, but was also a good learning opportunity!

@jameskerr jameskerr merged commit 19fa6af into master Dec 8, 2020
@jameskerr jameskerr deleted the tree_list branch December 8, 2020 05:03
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.

2 participants