Add support to make an item "hybrid" #86
Conversation
When testing new feature "hybrid" with dnd, the style for no background was being ignored. Moving it to the stylesheet makes it not visible even when doing dnd as expected.
WalkthroughThis change introduces hybrid node support to the organizational chart component by adding a Changes
Assessment against linked issues
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/flowingcode/vaadin/addons/orgchart/HybridDataPropertyDemo.java (1)
72-72: Fix typo in email address.There's a small typo in the email address for Charles Thomas.
- item6.setData("mail", "ctomas@example.com"); + item6.setData("mail", "cthomas@example.com");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pom.xml(1 hunks)src/main/java/com/flowingcode/vaadin/addons/orgchart/OrgChartItem.java(3 hunks)src/main/resources/META-INF/frontend/fc-orgchart.js(0 hunks)src/main/resources/META-INF/resources/frontend/fc-orgchart-styles.css(1 hunks)src/test/java/com/flowingcode/vaadin/addons/orgchart/HybridDataPropertyDemo.java(1 hunks)src/test/java/com/flowingcode/vaadin/addons/orgchart/HybridEnhancedChartDemo.java(1 hunks)src/test/java/com/flowingcode/vaadin/addons/orgchart/OrgchartDemoView.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/resources/META-INF/frontend/fc-orgchart.js
🔇 Additional comments (12)
src/main/java/com/flowingcode/vaadin/addons/orgchart/OrgChartItem.java (3)
31-31: Added Jackson annotation import for JSON property handling.The import for
JsonPropertyannotation is correctly added to support the serialization mapping in the new hybrid property.
49-49: New hybrid property added.The boolean field is appropriately defined for storing the hybrid state of an item.
136-158: Well-documented hybrid property methods.The implementation of the getter and setter methods for the hybrid property is clean and follows Java Bean conventions. The JavaDoc is thorough and clearly explains the purpose and behavior of the hybrid property.
The
@JsonProperty("isHybrid")annotation ensures proper JSON serialization naming, which is important for JavaScript interoperability.pom.xml (1)
5-5: Appropriate version bump for new feature.Increasing the minor version from 5.1.1-SNAPSHOT to 5.2.0-SNAPSHOT is appropriate for adding the new hybrid node feature.
src/main/resources/META-INF/resources/frontend/fc-orgchart-styles.css (1)
30-32: Fixed background styling issue.Adding this CSS rule to explicitly set
background-image: nonefor the.orgchartclass addresses the issue mentioned in the PR objectives where the "no background" style was being ignored after drag operations. Moving this style to the component's stylesheet ensures consistent styling behavior.src/test/java/com/flowingcode/vaadin/addons/orgchart/HybridEnhancedChartDemo.java (1)
67-67: Updated chart title for clarity.The title has been updated to clarify that this example demonstrates the vertical level property approach to hybrid charts, distinguishing it from the new hybrid data property approach.
src/test/java/com/flowingcode/vaadin/addons/orgchart/OrgchartDemoView.java (1)
43-43: LGTM! Demo registration added correctly.The new
HybridDataPropertyDemoclass is properly registered in the demo view, following the established pattern of the existing demo registrations.src/test/java/com/flowingcode/vaadin/addons/orgchart/HybridDataPropertyDemo.java (5)
30-34: LGTM! Class structure and annotations look good.The class is properly annotated with
@PageTitle,@DemoSource, and@Routeannotations, consistent with other demo classes in the codebase.
38-42: LGTM! Well-structured node template with conditional rendering.The node template nicely demonstrates the ability to conditionally display additional information (email) when present in the node's data.
46-46: Clarify the purpose of setChartNodeContent("title").You're setting the chart node content to "title" while also using a custom template that displays more than just the title. Consider adding a comment explaining the relationship between these two approaches, or whether this call is necessary given the custom template usage.
59-61: LGTM! Clear demonstration of hybrid property.Good use of descriptive comments to highlight where the hybrid property is being set, making the demo's purpose clear.
74-76: LGTM! Second hybrid node clearly indicated.The second demonstration of the hybrid property is well-commented, helping users understand where to look for the feature implementation.
When an item its set to "hybrid", the item's descendant children will be arranged vertically.
Close #85.
When testing this feature with drag and drop enabled, the no background style, was being ignore after the first drag. I move the rule to the stylesheet of the component to avoid this.
Summary by CodeRabbit
New Features
Style
Documentation