-
Notifications
You must be signed in to change notification settings - Fork 520
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
feat(TreeView): add count prop to TreeView.SubTree #2455
Conversation
Co-authored-by: Mike Perrotti <mperrotti@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 27c902e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
column-gap: 0.5rem; | ||
height: 2rem; | ||
|
||
@media (prefers-reduced-motion: no-preference) { |
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.
❤️
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.
Looks great! 🎉
I left a few comments, mostly about documentation and the storybook. The comment about merging the sx
prop is the only functional change we might need to make.
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
> | ||
{Array.from({length: count}).map((_, i) => { | ||
return <SkeletonItem aria-hidden={true} key={i} /> | ||
})} |
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 have a hunch that these skeleton items are overflowing the parent item since the parent item has a hardcoded height. We might need to adjust the height with the sx
prop to ensure the skeleton items don't overlap with items below.
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.
Just pushed up a fix that tackled this with minHeight
instead of height
, let me know if that would work or if we should use the sx prop instead!
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.
Looks good to merge after the coarse pointer height issue is fixed 👍
Co-authored-by: Cole Bemis <colebemis@github.com>
…to treeview-item-count-prop
* feat(TreeView): add count prop to TreeView.SubTree Co-authored-by: Mike Perrotti <mperrotti@users.noreply.github.com> * chore: add changeset * refactor(TreeView): update px units to rem * Update docs/content/TreeView.mdx Co-authored-by: Cole Bemis <colebemis@github.com> * Update .changeset/tender-turtles-serve.md Co-authored-by: Cole Bemis <colebemis@github.com> * refactor(TreeView): update stories and merge sx prop * refactor: update height for items and adjust height for coarse pointers * Update src/TreeView/TreeView.stories.tsx Co-authored-by: Cole Bemis <colebemis@github.com> * fix: update coarse pointer styles Co-authored-by: Mike Perrotti <mperrotti@users.noreply.github.com> Co-authored-by: Cole Bemis <colebemis@github.com>
Add the
count
prop toTreeView.SubTree
to display a skeleton loading state with the provided count of items.Shoutout to @mperrotti for the shimmer styles! 🥳
Screenshots
Screen.Recording.2022-10-19.at.2.28.39.PM.mov
Merge checklist