feat: add json-migration-helper for compatibility with Vaadin 25 #171
Conversation
|
Warning Rate limit exceeded@javier-godoy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughAdds a json-migration-helper dependency and bumps the addon version, updates .gitignore with generated/dev bundle/frontend entries, and refactors GoogleMaps Java classes to use JsonMigration helpers for event-data extraction and JSON property setting, plus minor formatting/import adjustments. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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. Comment |
![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)
pom.xml (1)
135-139: The SNAPSHOT dependency is available; consider versioning strategy for stability.The
json-migration-helper:0.0.1-SNAPSHOTartifact is confirmed to exist in the FlowingCode Snapshots repository. However, SNAPSHOT versions can still cause build reproducibility issues as they are non-deterministic by nature. Before merging to master, verify your team's dependency management strategy supports SNAPSHOT artifacts, or consider coordinating with the dependency maintainer to release a stable version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)pom.xml(2 hunks)src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java(9 hunks)src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java(18 hunks)src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java(3 hunks)
🔇 Additional comments (12)
pom.xml (1)
7-7: LGTM: Version bump is appropriate.The version bump from 2.3.1-SNAPSHOT to 2.4.0-SNAPSHOT correctly reflects the addition of new functionality for Vaadin 25 compatibility.
.gitignore (1)
12-18: LGTM: Appropriate ignore patterns for Vaadin 25.The added ignore patterns correctly exclude Vaadin 25 generated frontend files and bundles from version control.
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java (2)
196-198: LGTM: Consistent JsonMigration usage.The refactoring from
getElement().setPropertyJson()toJsonMigration.setPropertyJson()maintains the same behavior while using the new abstraction layer for Vaadin 25 compatibility.
206-208: LGTM: Consistent JsonMigration usage.The label property setting correctly uses the JsonMigration helper, consistent with the icon property handling.
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java (2)
190-196: LGTM: Deprecated method refactored consistently.The deprecated
setIcons(Icon...)method correctly uses JsonMigration for property setting, maintaining consistency even for deprecated APIs.
203-209: LGTM: Consistent JsonMigration usage.The current
setIcons(IconSequence...)method correctly uses the JsonMigration helper for property setting.src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java (6)
516-526: LGTM: Correct event data extraction with JsonMigration.The click listener correctly uses
JsonMigration.getEventData(ev)to extract the latLng data, maintaining compatibility with Vaadin 25.
528-538: LGTM: Consistent event data extraction.The right-click listener uses the same JsonMigration pattern for event data extraction, consistent with the click listener implementation.
765-774: LGTM: Consistent event data extraction in bounds changed listener.The bounds changed listener correctly uses JsonMigration for event data extraction, consistent with other event listeners.
799-808: LGTM: Deprecated method refactored consistently.The deprecated
addCustomControlsmethod correctly uses JsonMigration for property setting, maintaining consistency with the current API.
815-828: LGTM: Correct property setting with JsonMigration.The
setCustomControlsmethod correctly usesJsonMigration.setPropertyJson()for setting custom controls on the element.
922-929: LGTM: Consistent property setting in setMapStyle.The
setMapStylemethod correctly uses JsonMigration for property setting, consistent with other property-setting methods in the class.
javier-godoy
force-pushed
the
vaadin25
branch
from
3cc6a5a to
73691aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.gitignore (1)
19-19: Minor: Add leading slash for consistency.Line 19 uses
vite.config.tswithout a leading/, while most other entries use leading slashes. This will matchvite.config.tsin any directory rather than just the root. Consider adding a leading/for consistency if the intent is to ignore only the root-level file.-vite.config.ts +/vite.config.ts
javier-godoy
force-pushed
the
vaadin25
branch
from
73691aa to
17c9d22
Compare
javier-godoy
force-pushed
the
vaadin25
branch
from
17c9d22 to
d593c04
Compare
|
Summary by CodeRabbit