-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Initial implementation of collections API and Tree #31
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
Changes from all commits
6981b5b
b931db8
9725057
0ce60de
58997b3
968bad4
8fe701c
93d01cd
e8a70db
e247598
0085d9a
0c4520d
04dd466
7608e02
7a2c84f
be430bf
57aa310
b7ab2ff
8893475
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,9 @@ governing permissions and limitations under the License. | |
| padding: 0; | ||
| user-select: none; | ||
| outline: none; | ||
|
|
||
| height: 500px; | ||
| width: 200px; | ||
| } | ||
|
|
||
| .spectrum-TreeView-item { | ||
|
|
@@ -70,7 +73,7 @@ governing permissions and limitations under the License. | |
| content: ''; | ||
|
|
||
| position: absolute; | ||
| left: calc(0 + var(--spectrum-treeview-item-border-size)); | ||
| left: var(--spectrum-treeview-item-border-size); | ||
| right: 0; | ||
| z-index: -1; /* make sure we don't block clicks on chevron */ | ||
|
|
||
|
|
@@ -165,3 +168,8 @@ governing permissions and limitations under the License. | |
| /* topdoc | ||
| {{ treeview/treeview-disabled.yml }} | ||
| */ | ||
|
|
||
| .spectrum-TreeView-heading { | ||
| padding: calc(calc(var(--spectrum-treeview-item-margin-y) * 2) + var(--spectrum-treeview-item-padding-y)) 10px; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reasonably certain you can just do
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all temporary. I am guessing DNA will eventually have an exact value for this. Was just making something kinda close |
||
| font-weight: bold; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './src'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| { | ||
| "name": "@react-aria/collections", | ||
| "version": "3.0.0-alpha.1", | ||
| "description": "Spectrum UI components in React", | ||
| "main": "dist/main.js", | ||
| "module": "dist/module.js", | ||
| "types": "dist/types.d.ts", | ||
| "source": "src/index.ts", | ||
| "files": [ | ||
| "dist" | ||
| ], | ||
| "sideEffects": false, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/adobe/react-spectrum" | ||
| }, | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.6.2", | ||
| "@react-stately/collections": "^3.0.0-alpha.1", | ||
| "@react-types/shared": "^3.0.0-alpha.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": "^16.8.0", | ||
| "react-dom": "^16.8.0" | ||
| }, | ||
| "publishConfig": { | ||
| "access": "public" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import {Collection, Layout, LayoutInfo} from '@react-stately/collections'; | ||
| import {DOMProps} from '@react-types/shared'; | ||
| import React, {CSSProperties} from 'react'; | ||
| import {ScrollView} from './ScrollView'; | ||
| import {useCollectionState} from '@react-stately/collections'; | ||
|
|
||
| interface CollectionViewProps<T, V> extends DOMProps { | ||
| children: (type: string, content: T) => V, | ||
| layout: Layout<T>, | ||
| collection: Collection<T> | ||
| } | ||
|
|
||
| export function CollectionView<T, V>(props: CollectionViewProps<T, V>) { | ||
| let {children: renderView, layout, collection, ...otherProps} = props; | ||
| let { | ||
| visibleViews, | ||
| visibleRect, | ||
| setVisibleRect, | ||
| contentSize, | ||
| isAnimating, | ||
| collectionManager | ||
| } = useCollectionState({ | ||
| layout, | ||
| collection, | ||
| renderView, | ||
| renderWrapper: (reusableView) => ( | ||
| <div key={reusableView.key} role="presentation" style={layoutInfoToStyle(reusableView.layoutInfo)}> | ||
| {reusableView.rendered} | ||
| </div> | ||
| ) | ||
| }); | ||
|
|
||
| return ( | ||
| <ScrollView | ||
| {...otherProps} | ||
| innerStyle={isAnimating ? {transition: `none ${collectionManager.transitionDuration}ms`} : undefined} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why does a duration need to be passed with none? I think if we're trying to have no transition the preferred method is to set:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a trick to avoid needing to re-render all of the children when we want to enable/disable animations. Instead, the children all inherit the property from the parent and we only need to re-render one div. |
||
| contentSize={contentSize} | ||
| visibleRect={visibleRect} | ||
| onVisibleRectChange={setVisibleRect}> | ||
| {visibleViews} | ||
| </ScrollView> | ||
| ); | ||
| } | ||
|
|
||
| function layoutInfoToStyle(layoutInfo: LayoutInfo): CSSProperties { | ||
| return { | ||
| position: 'absolute', | ||
| overflow: 'hidden', | ||
| top: layoutInfo.rect.y, | ||
| left: layoutInfo.rect.x, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this be problematic for RTL?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah - the layout would handle that. We'd still position here with |
||
| transition: 'all', | ||
| WebkitTransition: 'all', | ||
| WebkitTransitionDuration: 'inherit', | ||
| transitionDuration: 'inherit', | ||
| width: layoutInfo.rect.width + 'px', | ||
| height: layoutInfo.rect.height + 'px', | ||
| opacity: layoutInfo.opacity, | ||
| zIndex: layoutInfo.zIndex, | ||
| transform: layoutInfo.transform, | ||
| contain: 'size layout style paint' | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import {DOMProps} from '@react-types/shared'; | ||
| import {flushSync} from 'react-dom'; | ||
| import React, {CSSProperties, ReactNode, useCallback, useEffect, useLayoutEffect, useRef, useState} from 'react'; | ||
| import {Rect, Size} from '@react-stately/collections'; | ||
|
|
||
| interface ScrollViewProps extends DOMProps { | ||
| contentSize: Size, | ||
| visibleRect: Rect, | ||
| onVisibleRectChange: (rect: Rect) => void, | ||
| children: ReactNode, | ||
| innerStyle: CSSProperties | ||
| } | ||
|
|
||
| export function ScrollView(props: ScrollViewProps) { | ||
| let {contentSize, visibleRect, onVisibleRectChange, children, innerStyle, ...otherProps} = props; | ||
| let ref = useRef<HTMLDivElement>(); | ||
| let state = useRef({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a confusing name for a variable, too much in React is wrapped up with state behavior and this does not do any of that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. heh, we have a few of these already too. We even have a prop named |
||
| scrollTop: 0, | ||
| scrollLeft: 0, | ||
| scrollEndTime: 0, | ||
| scrollTimeout: null, | ||
| width: 0, | ||
| height: 0 | ||
| }).current; | ||
|
|
||
| let [isScrolling, setScrolling] = useState(false); | ||
| let onScroll = useCallback((e) => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how does useCallback help here? wouldn't state.scrollEndTime cause that to be a bit pointless?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe - |
||
| flushSync(() => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm... flush the entire tree? how much is this affecting?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It synchronously flushes the updates made inside the |
||
| state.scrollTop = e.currentTarget.scrollTop; | ||
| state.scrollLeft = e.currentTarget.scrollLeft; | ||
| onVisibleRectChange(new Rect(state.scrollLeft, state.scrollTop, state.width, state.height)); | ||
|
|
||
| if (!isScrolling) { | ||
| setScrolling(true); | ||
| } | ||
|
|
||
| // So we don't constantly call clearTimeout and setTimeout, | ||
| // keep track of the current timeout time and only reschedule | ||
| // the timer when it is getting close. | ||
| let now = Date.now(); | ||
| if (state.scrollEndTime <= now + 50) { | ||
| state.scrollEndTime = now + 300; | ||
|
|
||
| clearTimeout(state.scrollTimeout); | ||
| state.scrollTimeout = setTimeout(() => { | ||
| setScrolling(false); | ||
| state.scrollTimeout = null; | ||
| }, 300); | ||
| } | ||
| }); | ||
| }, [isScrolling, onVisibleRectChange, state.height, state.scrollEndTime, state.scrollLeft, state.scrollTimeout, state.scrollTop, state.width]); | ||
|
|
||
| useEffect(() => { | ||
| // TODO: resize observer | ||
| // https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver | ||
| let updateSize = () => { | ||
| let dom = ref.current; | ||
| if (!dom) { | ||
| return; | ||
| } | ||
|
|
||
| let w = dom.offsetWidth; | ||
| let h = dom.offsetHeight; | ||
|
|
||
| if (state.width !== w || state.height !== h) { | ||
| state.width = w; | ||
| state.height = h; | ||
| onVisibleRectChange(new Rect(state.scrollLeft, state.scrollTop, w, h)); | ||
| } | ||
| }; | ||
|
|
||
| updateSize(); | ||
| window.addEventListener('resize', updateSize, false); | ||
| return () => { | ||
| window.removeEventListener('resize', updateSize, false); | ||
| }; | ||
| }, [onVisibleRectChange, state.height, state.scrollLeft, state.scrollTop, state.width]); | ||
|
|
||
| useLayoutEffect(() => { | ||
| let dom = ref.current; | ||
| if (!dom) { | ||
| return; | ||
| } | ||
|
|
||
| if (visibleRect.x !== state.scrollLeft) { | ||
| state.scrollLeft = visibleRect.x; | ||
| dom.scrollLeft = visibleRect.x; | ||
| } | ||
|
|
||
| if (visibleRect.y !== state.scrollTop) { | ||
| state.scrollTop = visibleRect.y; | ||
| dom.scrollTop = visibleRect.y; | ||
| } | ||
| }, [state.scrollLeft, state.scrollTop, visibleRect.x, visibleRect.y]); | ||
|
|
||
| return ( | ||
| <div {...otherProps} style={{position: 'relative', overflow: 'auto'}} ref={ref} onScroll={onScroll}> | ||
| <div style={{width: contentSize.width, height: contentSize.height, pointerEvents: isScrolling ? 'none' : 'auto', ...innerStyle}}> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isScrolling seems to hang around just a touch too long? when i scroll the tree, there's a noticeable pause between when things stop scrolling and when the hover event takes place
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's currently 300ms - same threshold as v2. but we can adjust |
||
| {children} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './CollectionView'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| { | ||
| "name": "@react-aria/tree", | ||
| "version": "3.0.0-alpha.1", | ||
| "description": "Spectrum UI components in React", | ||
| "main": "dist/main.js", | ||
| "module": "dist/module.js", | ||
| "types": "dist/types.d.ts", | ||
| "source": "src/useTooltip.ts", | ||
| "files": [ | ||
| "dist" | ||
| ], | ||
| "sideEffects": false, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/adobe/react-spectrum" | ||
| }, | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.6.2" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": "^16.8.0", | ||
| "react-dom": "^16.8.0" | ||
| }, | ||
| "devDependencies": { | ||
| "react": "^16.8.0", | ||
| "react-dom": "^16.8.0" | ||
| }, | ||
| "publishConfig": { | ||
| "access": "public" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './useTree'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // import React from 'react'; | ||
|
|
||
| export function useTree() { | ||
| return {}; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './src/index'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| { | ||
| "name": "@react-spectrum/tree", | ||
| "version": "3.0.0-alpha.1", | ||
| "description": "Spectrum UI components in React", | ||
| "main": "dist/main.js", | ||
| "module": "dist/module.js", | ||
| "types": "dist/types.d.ts", | ||
| "source": "src/index.ts", | ||
| "files": [ | ||
| "dist" | ||
| ], | ||
| "sideEffects": false, | ||
| "targets": { | ||
| "main": { | ||
| "includeNodeModules": [ | ||
| "@adobe/spectrum-css-temp" | ||
| ] | ||
| }, | ||
| "module": { | ||
| "includeNodeModules": [ | ||
| "@adobe/spectrum-css-temp" | ||
| ] | ||
| } | ||
| }, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/adobe/react-spectrum" | ||
| }, | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.6.2", | ||
| "@react-stately/tree": "^3.0.0-alpha.1", | ||
| "@spectrum-icons/ui": "^3.0.0-alpha.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": "^16.8.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@adobe/spectrum-css-temp": "^3.0.0-alpha.1", | ||
| "react": "^16.8.0" | ||
| }, | ||
| "publishConfig": { | ||
| "access": "public" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was testing stuff - guess it'll move to the storybook css eventually