fix(demo): show message if label text is missing when adding marker #162
Conversation
WalkthroughAdds an early return in AddMarkersDemo’s “Add Marker” handler: when “With Label” is checked and the label text is empty, a notification is shown and marker creation is aborted. Non-empty labels continue to create and style a MarkerLabel as before. No public APIs changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (2)
src/test/java/com/flowingcode/vaadin/addons/googlemaps/AddMarkersDemo.java (2)
122-131: Don’t abort marker creation on empty label; add unlabeled marker instead (per Issue #161).Issue #161’s expected behavior says: “If label is not defined or is empty, no label should be displayed.” The current early return prevents adding the marker at all when “With Label” is checked but the text is empty. Prefer letting the marker be created without a label, optionally notifying the user. Also treat whitespace-only input as empty and trim the label before applying.
Proposed refactor:
- if(withLabel.getValue()) { - if(labelText.getValue().isEmpty()) { - Notification.show("Please select a label for the marker"); - return; - } - MarkerLabel label = new MarkerLabel(labelText.getValue()); - label.setColor("white"); - label.setFontWeight("bold"); - marker.setLabel(label); - } + if (withLabel.getValue()) { + String text = Optional.ofNullable(labelText.getValue()).orElse(""); + if (text.isBlank()) { + // Inform but still add the marker without a label (matches Issue #161 expectation) + Notification.show("Please enter a label or uncheck 'With Label' to add an unlabeled marker"); + } else { + MarkerLabel label = new MarkerLabel(text.trim()); + label.setColor("white"); + label.setFontWeight("bold"); + marker.setLabel(label); + } + }If you intentionally prefer blocking the action, please confirm the UX decision and consider the minimal improvements below (whitespace handling + clearer message).
123-126: Handle whitespace-only labels and clarify the message.Use
isBlank()to catch whitespace-only input and improve the copy (“enter” vs “select”). If you keep the early return behavior, minimally do:- if(labelText.getValue().isEmpty()) { - Notification.show("Please select a label for the marker"); + if (labelText.getValue() == null || labelText.getValue().isBlank()) { + Notification.show("Please enter a label for the marker"); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/test/java/com/flowingcode/vaadin/addons/googlemaps/AddMarkersDemo.java(1 hunks)
github-project-automation
bot
moved this from To Do
to Pending release
in Flowing Code Addons
Close #161
Summary by CodeRabbit