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

Animated the Sidebar Objects Tree view opening/closing #9287

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ehconitin
Copy link
Contributor

closes #6485

2024-12-31.00-15-33.mov

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR adds smooth animations to the sidebar objects tree view by implementing a new expandable container component using Framer Motion for transitions.

  • Added new NavigationDrawerSubItemAnimatedExpandableContainer component with height/opacity animations and 300ms duration
  • Modified NavigationDrawerItemForObjectMetadataItem to use animated container for sub-items expansion
  • Updated CurrentWorkspaceMemberFavorites to animate folder content expansion/collapse
  • Animations use theme-consistent easing and timing (300ms) for a polished feel

3 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Member

@magrinj magrinj left a comment

Choose a reason for hiding this comment

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

Great job, some comments about the implementation and component naming ! 🙂

Copy link
Member

@magrinj magrinj left a comment

Choose a reason for hiding this comment

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

Thanks for the tests and the refactor, just adding a comment to clarify where the hook should take place

@magrinj magrinj requested a review from lucasbordeau January 2, 2025 09:57
@ehconitin ehconitin marked this pull request as draft January 2, 2025 10:01
@ehconitin
Copy link
Contributor Author

@magrinj
wdyt?
one generic expandable container. not sure if keeping the hook in the component is the right choice and not sure if we can have hooks in twenty-ui @lucasbordeau? If not I can take it out of twenty-ui but then I don't know which place would be better suited for it :)

@ehconitin ehconitin requested a review from magrinj January 2, 2025 12:54
@ehconitin ehconitin marked this pull request as ready for review January 2, 2025 12:54
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR continues the implementation of sidebar animations by introducing a new reusable AnimatedExpandableContainer component and refactoring existing components to use it. Here are the key changes:

  • Created new AnimatedExpandableContainer in twenty-ui package with Framer Motion animations and configurable dimensions/durations
  • Removed old static ExpandableContainer in favor of the new animated version
  • Refactored AdvancedSettingsWrapper and favorites components to use the new container
  • Added comprehensive Storybook stories demonstrating different animation configurations

The implementation looks solid and matches the desired 300ms animation requirement from the issue.

11 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile

@lucasbordeau
Copy link
Contributor

I think that putting it into twenty-ui is not critical for now, let's be cautious and extract utils, states and hook into the relevant files and folders, remember to keep one util per file, etc.

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

Successfully merging this pull request may close these issues.

Animate the Sidebar Objects Tree view opening/closing
4 participants