-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: Table column updating #8211
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
Conversation
@@ -1208,6 +1208,75 @@ function Example() { | |||
} | |||
``` | |||
|
|||
## Dependencies |
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.
name title of section?
Use the same strategies for reducing unnecessary invalidations as you would with any other hook. | ||
|
||
This prop is an array of dependencies that will trigger a cache invalidation within the collection. It should follow the same rules as React various hooks with dependencies arrays. | ||
> The list of dependencies must have a constant number of items and be written inline like `[dep1, dep2, dep3]`. |
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.
link to React docs? I don't know if we follow exactly the same algorithm for comparing
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.
We pass the dependencies directly to useMemo internally :)
<TableBody items={items} dependencies={[cols]}> | ||
{item => ( | ||
<Row id={item.id} columns={cols} dependencies={[cols]}> | ||
{(col) => { | ||
return <Cell>{item[col.id]}</Cell>; | ||
}} | ||
</Row> |
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.
this example isn't working for some reason??
<TableBody items={items} dependencies={[cols]}> | ||
{item => ( | ||
<Row id={item.id} columns={cols} dependencies={[cols]}> | ||
{(col) => { | ||
return <Cell>{item[col.id]}</Cell>; | ||
}} | ||
</Row> |
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.
this example isn't working for some reason??
|
||
|
||
if (this._minInvalidChildIndex === child && child.previousSibling) { | ||
this.invalidateChildIndices(child.previousSibling); |
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.
Shouldn't it move to the next sibling? After removing a child, the next sibling will now have a new index. The previous sibling won't change. Also what happens if you remove the first child?
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.
actually I see I already had that below. I think we can just set minInvalidChildIndex to null here.
Merged the dependencies example in Table docs into the Content section, which covers dynamic collections. Added checkbox group to show/hide columns, and button to add rows. |
made a separate story so it isn't included in the docs
…umn-updating # Conflicts: # packages/@react-aria/collections/src/Document.ts
## API Changes
@react-spectrum/s2/@react-spectrum/s2:TableBody TableBody <T extends {}> {
children?: ReactNode | (T) => ReactNode
+ dependencies?: ReadonlyArray<any>
items?: Iterable<T>
renderEmptyState?: (TableBodyRenderProps) => ReactNode
} /@react-spectrum/s2:Row Row <T extends {}> {
children?: ReactNode | (T) => ReactElement
columns?: Iterable<T>
+ dependencies?: ReadonlyArray<any>
id?: Key
textValue?: string
} /@react-spectrum/s2:TableBodyProps TableBodyProps <T> {
children?: ReactNode | (T) => ReactNode
+ dependencies?: ReadonlyArray<any>
items?: Iterable<T>
renderEmptyState?: (TableBodyRenderProps) => ReactNode
} /@react-spectrum/s2:RowProps RowProps <T> {
children?: ReactNode | (T) => ReactElement
columns?: Iterable<T>
+ dependencies?: ReadonlyArray<any>
id?: Key
textValue?: string
} |
Closes #8127, #7345
Combination of examples and docs and some fixes to Document. Some of the changes were borrowed from @LFDanLu 's PR https://github.com/adobe/react-spectrum/pull/7938/files
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: