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

[chore] - sparkle: enhance Tree component #6606

Merged
merged 5 commits into from
Jul 31, 2024
Merged

[chore] - sparkle: enhance Tree component #6606

merged 5 commits into from
Jul 31, 2024

Conversation

JulesBelveze
Copy link
Contributor

@JulesBelveze JulesBelveze commented Jul 31, 2024

Description

This PR enhances the Tree component in Sparkle to accommodate larger tree element sizes and improve its flexibility. The changes include:

  • Added a new size prop to Tree.Item with options "sm" (default) and "md" for different padding sizes.
  • Introduced an areActionsFading prop to control the visibility of action items.
  • Updated the visual prop to accept a ComponentType instead of a ReactNode, allowing for better type checking and consistency.
  • Improved the layout and styling of tree items, including better handling of icons, labels, and actions.
  • Updated the stories to demonstrate the new features and provide examples of usage with various data sources.

All tributes to @Duncid I just tested it and fixed the breaking front issues in #6607

Risk

Breaking UX

Deploy Plan

  • Deploy alpha sparkle and test any front breaking changes
  • Deploy sparkle

@JulesBelveze JulesBelveze marked this pull request as draft July 31, 2024 11:22
@JulesBelveze JulesBelveze marked this pull request as ready for review July 31, 2024 11:46
@JulesBelveze JulesBelveze requested a review from fontanierh July 31, 2024 12:14
@JulesBelveze JulesBelveze mentioned this pull request Jul 31, 2024
3 tasks
@JulesBelveze
Copy link
Contributor Author

These changes introduce a couple of breaking changes that are fixed in #6607

 - Increment package version for release of new features or fixes
 - Ensure consistency across package-lock.json and package.json files
Copy link
Contributor

@fontanierh fontanierh left a comment

Choose a reason for hiding this comment

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

LGTM but curious about the reasoning behind the type change of the visual prop

@@ -49,13 +49,15 @@ interface TreeItemProps {
label?: string;
type?: "node" | "item" | "leaf";
variant?: "file" | "folder" | "database" | "channel";
visual?: React.ReactNode;
size?: "sm" | "md";
visual?: ComponentType<{ className?: string }>;
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fontanierh Yeah I don't know either, this is actually what breaks in #6607.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno what was the rational behind but it could be to render all visuals as Icon and control the size?

Jules added 2 commits July 31, 2024 17:18
…isuals

 - Changed the `visual` prop type from `ComponentType` to `React.ReactNode` to allow for direct JSX elements
 - Updated `Tree.stories.tsx` to pass JSX elements directly instead of as references to components
…mponent

 - Cleaned up Tree.tsx by removing an unused import to streamline the component's dependencies
@JulesBelveze
Copy link
Contributor Author

FYI @fontanierh moved back to React.ReactNode.
A potential improvement here would be to restrict visual on our Icon type.

@JulesBelveze JulesBelveze merged commit 5752de8 into main Jul 31, 2024
4 checks passed
@JulesBelveze JulesBelveze deleted the TreeUpdate branch July 31, 2024 15:33
albandum pushed a commit that referenced this pull request Aug 28, 2024
* Updating tree to accomodate to bigger tree element sizes

* Changing copy

* [sparkle] - feature: bump version to 0.2.196

 - Increment package version for release of new features or fixes
 - Ensure consistency across package-lock.json and package.json files

* [sparkle] - refactor: update Tree component to accept ReactNode for visuals

 - Changed the `visual` prop type from `ComponentType` to `React.ReactNode` to allow for direct JSX elements
 - Updated `Tree.stories.tsx` to pass JSX elements directly instead of as references to components

* [sparkle] - refactor: remove unused ComponentType import from Tree component

 - Cleaned up Tree.tsx by removing an unused import to streamline the component's dependencies

---------

Co-authored-by: Edouard Wautier <[email protected]>
Co-authored-by: Jules <[email protected]>
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.

3 participants