Improvements: add mobile mode and other features - #68
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR bumps the project version, replaces MarkdownViewer rendering, adds mode and FAB model types, refactors ChatAssistant sizing and responsive behavior, updates frontend movement/resize support, and expands demos, tests, documentation, and metadata. ChangesChatAssistant 5.1 Feature Expansion
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js (1)
72-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCompute drag candidates against the FAB’s local bounds before mutating position.
Initial/reset placement uses offset-parent bounds, but drag/snap still use
window.innerWidth/innerHeightand viewport coordinates. For container-anchored FABs, this can jump or clamp the FAB outside its container. Also, Line 139 mutatespositionbefore the sensitivity check, so click-only jitter is later persisted bysnapToBoundary().🐛 Proposed fix
function fcChatAssistantBounds(item) { if (getComputedStyle(item).position === 'fixed' || !item.offsetParent) { - return { width: window.innerWidth, height: window.innerHeight }; + return { left: 0, top: 0, width: window.innerWidth, height: window.innerHeight }; } const rect = item.offsetParent.getBoundingClientRect(); - return { width: rect.width, height: rect.height }; + return { left: rect.left, top: rect.top, width: rect.width, height: rect.height }; } ... - let screenWidth = window.innerWidth; - let screenHeight = window.innerHeight; + let bounds = fcChatAssistantBounds(item); ... - screenWidth = window.innerWidth; - screenHeight = window.innerHeight; + bounds = fcChatAssistantBounds(item); ... - const xMax = Math.max(margin, screenWidth - itemRect.width - margin); - const yMax = Math.max(margin, screenHeight - itemRect.height - margin); + const xMax = Math.max(margin, bounds.width - itemRect.width - margin); + const yMax = Math.max(margin, bounds.height - itemRect.height - margin); ... - position.x = screenWidth - e.clientX - (itemRect.width / 2); - position.y = screenHeight - e.clientY - (itemRect.height / 2); + const nextX = bounds.left + bounds.width - e.clientX - (itemRect.width / 2); + const nextY = bounds.top + bounds.height - e.clientY - (itemRect.height / 2); // Do not move if delta is below sensitivity - if (isClickOnlyEvent()) { + if (Math.abs(nextX - initialPosition.x) < sensitivity + && Math.abs(nextY - initialPosition.y) < sensitivity) { return; } + position.x = nextX; + position.y = nextY; updatePosition();Also applies to: 103-115, 138-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js` around lines 72 - 84, The drag/snap logic in fc-chat-assistant-movement.js is still using window.innerWidth/window.innerHeight and viewport coordinates, which can push a container-anchored FAB outside its local bounds; update the candidate and boundary calculations in the movement handlers and snapToBoundary() to use the FAB’s local offset-parent bounds consistently, matching the initial/reset placement logic. Also adjust the drag update path so the position is not mutated until after the sensitivity check in the pointer/mouse move handler, preventing click jitter from being committed by snapToBoundary().
🧹 Nitpick comments (2)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.java (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the new listener bookkeeping in this roundtrip.
This still serializes only the message path. The PR added
screenSizeListeners/ScreenSizeListenerEntrystate toChatAssistant, but this test never registers one, so a listener-serialization regression would still pass here. Add at least oneaddScreenSizeListener(...)beforetestSerializationOf(chatAssistant).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.java` around lines 46 - 50, The roundtrip in SerializationTest.testSerialization only covers the message path and does not exercise the new screen-size listener state added to ChatAssistant. Update the test to register at least one listener via ChatAssistant.addScreenSizeListener(...) before calling testSerializationOf(chatAssistant), so ScreenSizeListenerEntry and screenSizeListeners are included in serialization coverage.src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java (1)
97-108: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider guarding the delayed
UI.accessagainst a detached UI.If the view is detached before the 5-second timer fires,
currentUI.access(...)can throwUIDetachedException. For demo robustness, you could capture the optional UI and no-op when absent, or useui.accessSynchronously-safe handling. Each click also spins up a freshjava.util.Timer(non-daemon) thread; reusing a single scheduler would be cleaner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java` around lines 97 - 108, The delayed UI update in ChatAssistantDemo should be made safe for detached views: the TimerTask currently calls UI.access on the captured UI without checking whether it is still attached, so guard this path using the existing UI capture in the click handler and skip the update when the UI is no longer available. While touching the same block, avoid creating a new java.util.Timer per click by centralizing scheduling/reusing a single scheduler for the delayed message logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 294-295: The repeated CSS/dimension literals in the ChatAssistant
setters are triggering duplication warnings, so extract the shared values into
constants and reuse them across the affected setters. Update the relevant
methods in ChatAssistant to reference a single source for the --fc-min-width,
--fc-min-height, and "width" strings so the dimension configuration stays
aligned and the static-analysis gate passes.
- Around line 134-135: The unread badge reset path is using a different default
text color than the initial badge style, so make the fallback in ChatAssistant
consistent. Update the DEFAULT_UNREAD_BADGE_COLOR used by
setUnreadBadgeColors(null, null) to match the initial unread badge text color
value, and verify the reset behavior in the unread badge styling logic remains
aligned with the existing default constants.
- Around line 451-471: Restore the per-UI singleton check in
ChatAssistant.onAttach so a Vaadin UI cannot end up with multiple ChatAssistant
instances registering competing listeners. Add a UI-scoped guard around the
existing addComponentRefreshedListener calls in ChatAssistant, and ensure that
any state used to track the active instance is cleared or updated when the
component is detached or the UI is reused. Keep the fix localized to
ChatAssistant.onAttach/onDetach and the related instance-tracking logic so only
one ChatAssistant can be active per UI at runtime.
- Around line 770-783: Separate the persisted FAB movable preference from the
runtime mobile override in ChatAssistant. Update setFabMovable(boolean) and the
mobile-mode handling around fabMovable so the stored preference is not
overwritten by the temporary mobile-state toggle, and isFabMovable() always
reflects the user’s preference rather than the effective drag state. Ensure the
mobile-mode path only adds or removes the client attribute for actual dragging
behavior without mutating the saved preference, and keep the relevant fab
element attribute updates consistent with that separation.
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 302-308: The bulk teardown in fcChatAssistantScreenSizeOffAll only
disconnects observers and leaves the per-key refresh guards behind, which can
block re-registration after detach/reattach. Update
fcChatAssistantScreenSizeOffAll to clear the same screen-size guard entries that
fcChatAssistantScreenSizeOff removes, alongside disconnecting each observer, so
ChatAssistant.onAttach can safely re-register listeners after a full teardown.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js`:
- Around line 29-32: The resize handler in fc-chat-assistant-resize.js is
caching overlay/content-part nodes too aggressively, so after the popover closes
and reopens it can keep targeting detached DOM references. Update fetchOverlay()
in the resize handle initialization path to re-resolve the current overlay and
the [part="content"] element on each open (or whenever the existing nodes are no
longer connected), and make sure shouldDrag(), setContentWidth(), and
setContentHeight() always operate on the refreshed nodes rather than stale
closures.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantFabConfigDemo.java`:
- Around line 63-75: The color selection buttons in ChatAssistantFabConfigDemo
should replace the current FAB color theme instead of accumulating multiple
variants. Update the handlers for the Success, Error, and Contrast buttons to
first remove the other color-related variants via
chatAssistant.removeFabThemeVariants(...) and then apply the chosen one with
chatAssistant.addFabThemeVariants(...), keeping the existing clearColors action
as-is. This ensures the demo always reflects the currently selected color
deterministically.
---
Outside diff comments:
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 72-84: The drag/snap logic in fc-chat-assistant-movement.js is
still using window.innerWidth/window.innerHeight and viewport coordinates, which
can push a container-anchored FAB outside its local bounds; update the candidate
and boundary calculations in the movement handlers and snapToBoundary() to use
the FAB’s local offset-parent bounds consistently, matching the initial/reset
placement logic. Also adjust the drag update path so the position is not mutated
until after the sensitivity check in the pointer/mouse move handler, preventing
click jitter from being committed by snapToBoundary().
---
Nitpick comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java`:
- Around line 97-108: The delayed UI update in ChatAssistantDemo should be made
safe for detached views: the TimerTask currently calls UI.access on the captured
UI without checking whether it is still attached, so guard this path using the
existing UI capture in the click handler and skip the update when the UI is no
longer available. While touching the same block, avoid creating a new
java.util.Timer per click by centralizing scheduling/reusing a single scheduler
for the delayed message logic.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.java`:
- Around line 46-50: The roundtrip in SerializationTest.testSerialization only
covers the message path and does not exercise the new screen-size listener state
added to ChatAssistant. Update the test to register at least one listener via
ChatAssistant.addScreenSizeListener(...) before calling
testSerializationOf(chatAssistant), so ScreenSizeListenerEntry and
screenSizeListeners are included in serialization coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c05eb13-8aae-47cb-a9dc-aaf4a8211467
📒 Files selected for processing (28)
.gitignoreREADME.mdpom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/ChatAssistantMode.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabPosition.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/Message.javasrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.jssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-message-styles.csssrc/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.javasrc/test/java/com/flowingcode/vaadin/addons/DemoLayout.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantBoxDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantFabConfigDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantModeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomChatMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/DemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/BasicIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/ViewIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.java
💤 Files with no reviewable changes (7)
- src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/Message.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/BasicIT.java
- src/test/java/com/flowingcode/vaadin/addons/DemoLayout.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomChatMessage.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomMessage.java
- src/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/DemoView.java
mlopezFC
left a comment
mlopezFC
left a comment
There was a problem hiding this comment.
Code Review
Solid, well-documented feature work: FAB drag/corner positioning, resizable chat window with min/max bounds, responsive desktop/mobile modes with opt-in breakpoint switching, screen-size listeners, and the migration to the native Vaadin Markdown component. The Java↔JS contract is clean and the inline comments explaining the popover/overlay sizing are genuinely helpful. Version bump 5.0.2 → 5.1.0 (MINOR) is correctly aligned with the feat: commits.
Most of the substantive points are left as inline comments. A few cross-cutting notes below.
Should address
- Missing
@sincetags on the new public/protected API (see inline comment onChatAssistantMode). FC add-on conventions require it for all new API elements.
Suggestions (non-blocking)
- Test coverage. There are no unit tests for the new pure-Java logic (
parseFabMarginfallbacks,setUnreadMessagesclamping to 0–99, size-variant add/remove resizing,addScreenSizeListenerargument validation, thesetModeopen-state reconciliation). Not a blocker — just flagging it as a nice-to-have. - Commit atomicity.
d38f33bbundles three logical changes (markdown component swap +markdown-editor-addonremoval + pom version bump). Ideally these would be separate commits (in particular a standalone version-bump commit). Suggestion only.
Minor / style
- Google Java Style: 11 occurrences of
if(/else if(without a space and 2 trailing-whitespace lines (ChatAssistant.java:922,940). Runningmvn com.spotify.fmt:fmt-maven-plugin:formatwould normalize these. ChatAssistant.java:chatWindow.setOpenOnClick(false)is set twice (insetUIand again ininitializeChatWindow) — harmless redundancy.
What looks good
- The native
Markdownmigration is clean — no leftoverMarkdownViewer/markdown-editorreferences, andvaadin-markdown-flowis present for the pinned Vaadin 24.8.0. ScreenSizeListenerEntry implements Serializableand the other new fields are serialization-safe.- Live-read
isResizable()/shouldDrag()letssetWindowResizabletake effect without re-init. - Auto-switching is opt-in to avoid a breaking change — good backward-compat judgment, well documented.
- README rewrite is a real improvement and drops the stale
toggle()/addChatSentListenerAPI.
| * The display mode of the chat assistant: {@link #MOBILE} opens the chat window as a full-screen | ||
| * dialog, while {@link #DESKTOP} opens it as an anchored popover. | ||
| */ | ||
| public enum ChatAssistantMode { |
There was a problem hiding this comment.
Missing @since tag. FlowingCode add-on conventions require @since on every new public/protected API element. This applies across the whole PR, not just this enum — please add @since 5.1.0 to:
- both new enums (
ChatAssistantMode,FabPosition), - the new events/enums (
ModeChangedEvent,ScreenSizeEvent,ScreenSizeDirection), - and the new
ChatAssistantmethods:setMode/getMode/setMobileMode/isMobileMode/addModeChangedListener,setFabPosition/getFabPosition/resetFabPosition,addFabThemeVariants/removeFabThemeVariants,setFabAnchoredToViewport/isFabAnchoredToViewport,setResizeIndicatorsVisible/isResizeIndicatorsVisible, thesetWindowMin*/Max*/Width/Heightfamily,addScreenSizeListener,setUnreadMessages/getUnreadMessages/setUnreadBadgeColors,setMobileModeSwitchingEnabled/isMobileModeSwitchingEnabled/getMobileBreakpoint.
Place it after the main description, before any @param/@return.
There was a problem hiding this comment.
@coderabbitai Please check if this comment has been addressed.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
model_dir="src/main/java/com/flowingcode/vaadin/addons/chatassistant/model"
assistant_file="src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java"
echo '=== Relevant Java source files ==='
fd -t f -e java . src/main/java/com/flowingcode/vaadin/addons/chatassistant | sort
echo
echo '=== Model/event declarations and `@since` context ==='
rg -n -C 8 \
'public (enum|class) (ChatAssistantMode|FabPosition|ModeChangedEvent|ScreenSizeEvent|ScreenSizeDirection)\b|`@since`\b' \
"$model_dir" "$assistant_file" || true
echo
echo '=== Requested ChatAssistant method declarations and `@since` context ==='
rg -n -C 10 \
'public .*\b(setMode|getMode|setMobileMode|isMobileMode|addModeChangedListener|setFabPosition|getFabPosition|resetFabPosition|addFabThemeVariants|removeFabThemeVariants|setFabAnchoredToViewport|isFabAnchoredToViewport|setResizeIndicatorsVisible|isResizeIndicatorsVisible|setWindow(?:Min|Max)?(?:Width|Height)|addScreenSizeListener|setUnreadMessages|getUnreadMessages|setUnreadBadgeColors|setMobileModeSwitchingEnabled|isMobileModeSwitchingEnabled|getMobileBreakpoint)\s*\(' \
"$assistant_file" || trueLength of output: 78601
@javier-godoy, this is partially addressed, but the comment should remain unresolved.
@since 5.1.0 was added to the enums, events, and most listed APIs. However, it is still missing from these explicitly requested methods in ChatAssistant.java:
setWindowMinWidth(String)— Line 984setWindowMinHeight(String)— Line 1017setWindowMaxWidth(String)— Line 1028setWindowMaxHeight(String)— Line 1050setWindowHeight(String)— Line 1072setWindowWidth(String)— Line 1091getUnreadMessages()— Line 1439setUnreadMessages(int)— Line 1449
Also, where present, the new @since tags are generally placed after @param/@return (for example, setMode at Line 1492), while the original request specified placing @since immediately after the description and before other Javadoc tags.
| public void testSerialization() throws ClassNotFoundException, IOException { | ||
| try { | ||
| ChatAssistant chatAssistant = new ChatAssistant(); | ||
| ChatAssistant<Message> chatAssistant = new ChatAssistant<Message>(); |
There was a problem hiding this comment.
Minor: the style guide prefers diamond syntax — new ChatAssistant<>() instead of new ChatAssistant<Message>().
There was a problem hiding this comment.
@coderabbitai Please check if this comment has been addressed.
There was a problem hiding this comment.
@javier-godoy The comment has been addressed. SerializationTest.java now uses new ChatAssistant<>().
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
You are interacting with an AI system.
96035c5 to
2025dd3
Compare
|
To split the markdown migration and the version bump I had to create 3 commits and do a force push. The other changes are left as WIP |
javier-godoy
left a comment
javier-godoy
left a comment
There was a problem hiding this comment.
Add-ons compatible with Vaadin 25 must support both Lumo and Aura themes.
github-project-automation
Bot
moved this from To Do
to In Progress
in Flowing Code Addons
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js (1)
103-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClamp anchored-container FABs against their container, not the viewport.
fcChatAssistantBounds()handles fixed vs offset-parent geometry, butsnapToBoundary()still usesscreenWidth/screenHeight. ForsetFabAnchoredToViewport(false), a container resize can leave a corner-positioned FAB outside its container.Proposed fix
function snapToBoundary() { // Get current dimensions to account for transforms const itemRect = fab.getBoundingClientRect(); + const bounds = fcChatAssistantBounds(item); - const xMax = Math.max(margin, screenWidth - itemRect.width - margin); - const yMax = Math.max(margin, screenHeight - itemRect.height - margin); + const xMax = Math.max(margin, bounds.width - itemRect.width - margin); + const yMax = Math.max(margin, bounds.height - itemRect.height - margin);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js` around lines 103 - 115, The snapToBoundary() logic is still clamping against screenWidth and screenHeight, so anchored FABs can escape their container when setFabAnchoredToViewport(false) is used. Update snapToBoundary() in fc-chat-assistant-movement.js to use the container-aware bounds from fcChatAssistantBounds() (or the same offset-parent geometry it returns) instead of viewport dimensions, and keep the existing position.x/position.y clamping and updatePosition() flow intact.
🧹 Nitpick comments (2)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.java (1)
54-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a Java text block for the sample Markdown string.
Same as noted elsewhere: this multi-line concatenation could be a text block for readability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.java` around lines 54 - 58, The sample Markdown string in ChatAssistantMarkdownDemo is built with multi-line concatenation, which should be simplified for readability. Update the message.setValue usage to use a Java text block instead of repeated string concatenation, keeping the same Markdown content and preserving the existing sample formatting.Source: Linters/SAST tools
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java (1)
132-260: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider Java text blocks for the multi-line dialogue strings.
SonarCloud flags several multi-line
+-concatenated strings here (e.g. lines 139-149, 173-178, 191-193, 199-202, 209-215, 240-243, 248-258) that could be simplified with text blocks ("""...""") for readability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java` around lines 132 - 260, The multi-line dialogue literals in createMessages() are still built with repeated string concatenation, which makes the sample data hard to read and maintain. Update the affected Message.builder().content(...) calls in ChatAssistantLazyLoadingDemo to use Java text blocks for each multi-line Hamlet excerpt, keeping the same text and line breaks while removing the chained + concatenations. Focus on the long content strings used for Claudius and Hamlet entries so the sample conversation remains identical but much clearer.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 928-930: `isFabMovable()` is currently reporting the raw
`fabMovable` flag even when container-anchored mode disables dragging, so align
it with the actual drag state used by the client script. Update
`ChatAssistant.isFabMovable()` to also account for the anchored condition
managed by `setFabAnchoredToViewport(boolean)` and the `anchored` client
attribute, ensuring it only returns true when dragging is actually possible.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java`:
- Around line 98-128: The ChatAssistantDemo click handler creates a new Timer
for each "Chat With Thinking" action, but the Timer is never canceled after the
one-shot task finishes, leaving a live background thread behind. Update the
logic around the delayed message task in the chatWithThinking listener to retain
the Timer instance and call cancel() (and purge if needed) once currentUI.access
updates delayedMessage and chatAssistant.updateMessage complete, so the timer
thread terminates after the scheduled run.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java`:
- Around line 111-146: The async demo in ChatAssistantGenerativeDemo is blocking
the shared ForkJoinPool.commonPool() by using CompletableFuture.runAsync without
an executor together with the sleep inside the delayed message flow. Update the
Chat With Generative Thinking click handler to run the background work on a
dedicated executor or UI-friendly task runner instead of the common pool, and
keep the UI updates through currentUI.access as they are. Ensure the streaming
logic around streamWords and delayedMessage remains the same, but remove any
blocking waits from the shared pool path.
---
Outside diff comments:
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 103-115: The snapToBoundary() logic is still clamping against
screenWidth and screenHeight, so anchored FABs can escape their container when
setFabAnchoredToViewport(false) is used. Update snapToBoundary() in
fc-chat-assistant-movement.js to use the container-aware bounds from
fcChatAssistantBounds() (or the same offset-parent geometry it returns) instead
of viewport dimensions, and keep the existing position.x/position.y clamping and
updatePosition() flow intact.
---
Nitpick comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java`:
- Around line 132-260: The multi-line dialogue literals in createMessages() are
still built with repeated string concatenation, which makes the sample data hard
to read and maintain. Update the affected Message.builder().content(...) calls
in ChatAssistantLazyLoadingDemo to use Java text blocks for each multi-line
Hamlet excerpt, keeping the same text and line breaks while removing the chained
+ concatenations. Focus on the long content strings used for Claudius and Hamlet
entries so the sample conversation remains identical but much clearer.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.java`:
- Around line 54-58: The sample Markdown string in ChatAssistantMarkdownDemo is
built with multi-line concatenation, which should be simplified for readability.
Update the message.setValue usage to use a Java text block instead of repeated
string concatenation, keeping the same Markdown content and preserving the
existing sample formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f425be89-a84e-4f7f-8535-20d6b08edbc8
⛔ Files ignored due to path filters (2)
src/main/resources/META-INF/resources/icons/chatbot.svgis excluded by!**/*.svgsrc/test/resources/META-INF/resources/chatbot.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.gitignoreREADME.mdpom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/ChatAssistantMode.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabPosition.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabVariant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/Message.javasrc/main/resources/META-INF/VAADIN/package.propertiessrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.jssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-message-styles.csssrc/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.javasrc/test/java/com/flowingcode/vaadin/addons/DemoLayout.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantBoxDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantFabConfigDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLogicTest.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantModeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomChatMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/DemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/AbstractViewTest.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/BasicIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/ViewIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatAssistantElement.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatBubbleElement.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.javasrc/test/resources/META-INF/frontend/styles/chat-assistant-styles-demo.css
💤 Files with no reviewable changes (1)
- src/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.java
✅ Files skipped from review due to trivial changes (14)
- .gitignore
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatAssistantElement.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/DemoView.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatBubbleElement.java
- src/test/resources/META-INF/frontend/styles/chat-assistant-styles-demo.css
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/AbstractViewTest.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomChatMessage.java
- src/main/resources/META-INF/VAADIN/package.properties
- src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/Message.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/BasicIT.java
- src/test/java/com/flowingcode/vaadin/addons/DemoLayout.java
- README.md
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLogicTest.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomMessage.java
🚧 Files skipped from review as they are similar to previous changes (12)
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.java
- src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/ChatAssistantMode.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java
- src/main/resources/META-INF/resources/frontend/styles/fc-chat-message-styles.css
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/it/ViewIT.java
- src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabPosition.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantBoxDemo.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantFabConfigDemo.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantModeDemo.java
- src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css
- pom.xml
- src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
| @@ -86,101 +117,208 @@ public class ChatAssistant<T extends Message> extends Div { | |||
| protected final Div resizerTopLeft = new Div(); | |||
|
|
|||
| protected static final int DEFAULT_FAB_SIZE = 60; | |||
| protected static final int DEFAULT_FAB_SMALL_SIZE = 50; | |||
| protected static final int DEFAULT_FAB_LARGE_SIZE = 72; | |||
| protected static final boolean DEFAULT_FAB_ANCHORED_TO_VIEWPORT = true; | |||
| protected static final boolean DEFAULT_RESIZE_INDICATORS_VISIBLE = false; | |||
| protected static final boolean DEFAULT_WINDOW_RESIZABLE = true; | |||
| protected static final boolean DEFAULT_FAB_MOVABLE = true; | |||
| protected static final ChatAssistantMode DEFAULT_MODE = ChatAssistantMode.DESKTOP; | |||
| protected static final int DEFAULT_MOBILE_BREAKPOINT = 768; | |||
| protected static final int DEFAULT_FAB_ICON_SIZE = 40; | |||
| protected static final int DEFAULT_FAB_MARGIN = 25; | |||
| protected static final int DEFAULT_RESIZER_SIZE = 25; | |||
| protected static final int DEFAULT_MAX_RESIZER_SIZE = 200; | |||
| protected static final int DEFAULT_DRAG_SENSITIVITY = 25; | |||
| protected static final FabPosition DEFAULT_POSITION = FabPosition.BOTTOM_RIGHT; | |||
| protected static final String DEFAULT_UNREAD_BADGE_BACKGROUND = "var(--lumo-warning-color)"; | |||
| protected static final String DEFAULT_UNREAD_BADGE_COLOR = "var(--lumo-warning-text-color)"; | |||
|
|
|||
| protected static final int DEFAULT_CONTENT_MIN_WIDTH = 150; | |||
| protected static final int DEFAULT_CONTENT_MIN_HEIGHT = 150; | |||
| protected static final String DEFAULT_POPOVER_TAG = "fc-chat-assistant-popover"; | |||
| protected static final String DEFAULT_DIALOG_TAG = "fc-chat-assistant-dialog"; | |||
| protected static final String DEFAULT_FAB_CLASS = "fc-chat-assistant-fab"; | |||
| protected static final String DEFAULT_RESIZE_CLASS = "fc-chat-assistant-resize"; | |||
| protected static final String RESIZE_INDICATOR_VISIBLE_CLASS = | |||
| "fc-chat-assistant-resize-indicator-visible"; | |||
| protected static final String DEFAULT_UNREAD_BADGE_CLASS = "fc-chat-assistant-unread-badge"; | |||
| protected static final String DEFAULT_FAB_ICON_SRC = loadDefaultFabIconSrc(); | |||
|
|
|||
| protected final VirtualList<T> content = new VirtualList<>(); | |||
| protected final List<T> messages = new ArrayList<>(); | |||
|
|
|||
| protected Component headerComponent; | |||
| protected Component footerContainer; | |||
| protected MessageInput messageInput; | |||
| protected Span whoIsTyping; | |||
| protected Registration defaultSubmitListenerRegistration; | |||
| protected int unreadMessages = 0; | |||
|
|
|||
| private int screenSizeKeySeq = 0; | |||
| private final Map<Integer, ScreenSizeListenerEntry> screenSizeListeners = new HashMap<>(); | |||
There was a problem hiding this comment.
Do we need to introduce all this protected API in a non-final class? Can it be made private instead?
There was a problem hiding this comment.
Update: I left the variables that were previously protected as-is and the newly introduced variables are all private now
There was a problem hiding this comment.
Partially addressed — caveat. The newly-introduced fields are now private and the pre-existing protected ones are unchanged; the eight protected resizer fields were also removed in favor of a private Map. However, several newly-introduced protected methods remain, none of which existed before: onMobileModeChange, onScreenSizeChange, createDefaultFabIcon, setFabSize, isMobile, and setMode(mode, fromClient). setFabSize and isMobile in particular have no obvious override rationale and could be private. So the field-visibility concern is resolved, but the broader protected-method surface is not.
🤖 This reply was generated with Claude Code.
There was a problem hiding this comment.
Trimmed the protected surface: setFabSize, isMobile, and createDefaultFabIcon are now private (commit 2e5c505) — none had an override rationale or external callers. onMobileModeChange/onScreenSizeChange stay accessible (they're @ClientCallable) and setMode(mode, boolean) remains overridable.
|
|
||
| /** | ||
| * Sets the number of unread messages to be displayed in the chat assistant. | ||
| * Sets the number of unread messages shown on the FAB badge. The value is clamped to the 0–99 |
There was a problem hiding this comment.
This change improves the documentation. Don't piggyback in unrelated commit.
There was a problem hiding this comment.
Kept the change; not rewriting the base commits (WIP-on-top, avoiding a force-push). Can amend + force-push if you prefer it isolated.
| * @param background the background color of the unread badge | ||
| * @param color the text color of the unread badge | ||
| */ | ||
| public void setUnreadBadgeColors(String background, String color) { |
There was a problem hiding this comment.
This method seems to correspond to another feature.
There was a problem hiding this comment.
This is the unread-badge-colors feature; kept it but not rewriting the base commits (WIP-on-top, avoiding a force-push). Can amend + force-push into its own commit if you prefer.
javier-godoy
left a comment
javier-godoy
left a comment
There was a problem hiding this comment.
🤖 Automated code review by Claude Code
15 verified findings from an xhigh-effort multi-agent review of this PR's diff against master (correctness regressions first, then maintainability cleanups). Each inline comment was produced by one finder agent and independently verified by a separate agent. CONFIRMED = reproduced from the code; PLAUSIBLE = mechanism verified but needs a runtime check. This is machine-generated — please confirm before acting.
| * @param width the width as an absolute CSS length | ||
| * @since 5.1.0 | ||
| */ | ||
| public void setWindowWidth(String width) { |
There was a problem hiding this comment.
🤖 Claude Code review — CONFIRMED (correctness)
setWindowWidth/Height(String) no longer accept relative CSS units
The new path stores the raw value and the resize script's clamp() does parseFloat(value) then re-appends 'px', so relative units like '50%' / '30vw' are silently reinterpreted as pixels and clamped to the min bound. Master called overlay.setWidth(width), which accepted any CSS length.
Failure scenario: setWindowWidth("50%") previously opened the window at half the viewport width. Now clamp parses 50 from "50%", and because the default min width is 150px it returns "150px" — the window renders as a fixed 150px box instead of 50% of the viewport.
Introduced in 9570271 — feat: add FAB, window, and mode configuration to ChatAssistant.
Two-commit interaction: the server-side rerouting is here (9570271); the parseFloat(...)+'px' clamp that drops the unit lives in fcChatAssistantApplyConstraints, added in 1d79af0 ("add chat window resize and sizing frontend").
🤖 Generated by Claude Code (Opus 4.8). Automated — verify before acting; PLAUSIBLE items need a runtime check.
There was a problem hiding this comment.
The String size setters now validate and reject non-pixel lengths up front with a clear message, instead of silently mis-clamping. Commit 2d25b21.
There was a problem hiding this comment.
Addressed, with a semantics caveat. The String size setters now validate and reject non-pixel lengths up front via requirePixelLength (throws IllegalArgumentException with a clear message), so the silent mis-clamping is gone. Note this is a behavioral change from master/5.0.x, which accepted any CSS length (%, vw, …): such calls now throw rather than being honored. Worth a conscious sign-off if any caller relied on relative units.
🤖 This reply was generated with Claude Code.
There was a problem hiding this comment.
The silent-unit bug is real, but note that rejecting non-px lengths (2d25b21) narrows the pre-existing setWindowWidth/Height(String) contract — on master these called overlay.setWidth/Height(...) and accepted any CSS length.
I think we can restore that contract without giving up bound enforcement. fcChatAssistantApplyConstraints already writes the bounds as inline CSS on the content part:
if (minWidth) contentPart.style.minWidth = minWidth;
if (maxWidth) contentPart.style.maxWidth = maxWidth;so CSS clamps the rendered size regardless of the width's unit — and CSS already applies "min wins over max when they cross", the same rule clamp re-implements. The only size consumer, drag-resize, computes from getBoundingClientRect() (the rendered, already-clamped box), never the inline width string.
That makes the parseFloat(value) + 'px' step in clamp largely redundant with the CSS min/max it sets right alongside it — and that step is exactly what strips %/vw. If clamp passed non-px values through verbatim:
const clamp = (valueRaw, minProp, maxProp) => {
// Only numerically clamp absolute px lengths; relative units (%/vw/…) are left to the
// CSS min/max-width already applied to the content part.
if (!/^\s*\d*\.?\d+(px)?\s*$/.test(valueRaw)) {
return valueRaw;
}
const value = parseFloat(valueRaw);
...
};then setWindowWidth("50%") would render at 50% bounded by the px min/max, and requirePixelLength could be dropped — restoring master's behavior. Worth a runtime check that % resolves against the expected containing block, but it looks feasible.
There was a problem hiding this comment.
Dropped requirePixelLength and its call sites, restoring master's contract of accepting any CSS length on the String size setters (commit f5d1026). As you suggested, the JS clamp now only numerically clamps plain px values and passes other units (%/vw/…) through verbatim to the CSS min/max already applied to the content part. Investigation confirmed % can't resolve meaningfully inside the shrink-to-fit popover, so rather than throw I documented that absolute or viewport-relative units (px/vw/vh) are recommended.
There was a problem hiding this comment.
Some extra context on the percentage case, since it informs whether the px-only check was actually a breaking change.
Why % can't work here. The chat window is a Popover, and its size is applied to the popover's ::part(content), which the CSS leaves width-less (display:flex; max-width:100%). So the overlay is shrink-to-fit — sized by its content, with no definite width of its own. CSS resolves a percentage width against the content-box width of the containing block, but that containing block is exactly this shrink-to-fit part: a circular dependency. The browser resolves the percentage to ~0 and the window falls back to its min-width floor (DEFAULT_CONTENT_MIN_WIDTH = 150px). 70% never means "70% of the viewport." (vw/vh are the opposite — viewport-relative, independent of the containing block — so 70vw genuinely works.)
It behaved the same on master. There setWindowWidth(String) did overlay.setWidth(width) on the very same Div nested inside the same shrink-to-fit popover, and that Div also carried min-width: 150px. So setWindowWidth("70%") on master resolved 70% against the same indefinite containing block, collapsed, and clamped to ~150px — not "70% of anything." The branch only changed where the value is applied (a --fc-* custom property → the content part); the old JS also silently did parseFloat("70%") → "70px". In neither version did a percentage produce a usable window size.
So the px restriction wasn't a behavioral break — it rejected an input that never worked. It was a contract-level narrowing at most (the String overload previously accepted any value without throwing), which is the reason I dropped it (commit f5d1026) rather than keep it: a hard throw would also reject vw/vh, which do work. Current behavior: accept any unit, px/vw/vh work, % is documented as unsupported — no behavioral regression and no contract narrowing.
| String maxWidth, | ||
| String maxHeight) { | ||
| if (messages != null) { | ||
| this.messages.addAll(messages); |
There was a problem hiding this comment.
🤖 Claude Code review — CONFIRMED (correctness)
(List, boolean) constructor copies the list instead of holding the caller's reference
The constructor now does this.messages.addAll(messages) into an owned internal ArrayList instead of retaining the caller's reference (master did this.messages = messages), so the ListDataProvider is no longer backed by the caller's list.
Failure scenario: A caller keeps a reference (List shared = new ArrayList<>(); new ChatAssistant<>(shared, false);), later appends to shared, then triggers a refresh (sendMessage(...) / refreshContent()). Previously refreshAll re-read the shared list and rendered the appended messages; now they never appear.
Introduced in 9570271 — feat: add FAB, window, and mode configuration to ChatAssistant.
🤖 Generated by Claude Code (Opus 4.8). Automated — verify before acting; PLAUSIBLE items need a runtime check.
There was a problem hiding this comment.
Intentional — we keep the defensive copy for safer encapsulation; documented in the constructor javadoc. No change.
There was a problem hiding this comment.
Follow-up after a closer look — the defensive copy is the right default, so no objection to keeping it, but a couple of points are worth capturing.
Why it's sound: there's no getter exposing the list, so this constructor was the only place the component's backing store was aliased to caller-held code. initializeContent wraps that exact instance in the default ListDataProvider (content.setItems(this.messages)) and sendMessage mutates it, so under master a caller holding the reference could clear()/reorder/remove out of band and desync the VirtualList key-mapper from the rendered rows. The copy closes that hazard and makes all three constructors consistent (the no-arg ones already own a fresh ArrayList).
The one caveat: this is a silent behavioral change from 5.0.x — the "pass a list, keep the reference, mutate it externally, call refreshContent()" pattern now no-ops with no compile error and no exception. The javadoc note ("The messages are copied; later changes to the supplied list are not reflected") is the right mitigation, but for a behavioral break in code that still compiles it deserves a release-note / migration-guide mention, and the javadoc should point callers who need externally-driven data at setDataProvider(...) (unaffected by the copy).
Minor, related: on the default-provider path public refreshContent() is now largely redundant (the only way to change default data is sendMessage, which already calls it); it stays meaningful for the setDataProvider path.
None of this needs to block the PR — the release-note/javadoc wording and the refreshContent() tidy-up can be handled in a later issue.
🤖 This reply was generated with Claude Code.
There was a problem hiding this comment.
Acknowledged — this is intentional. The initial-messages list is now defensively copied (see the constructor Javadoc: "The messages are copied; later changes to the supplied list are not reflected"), which also stops sendMessage(...) from leaking internal writes back into the caller-owned list. Not reverting to the old aliasing behavior.
That said, it is a source-compatible but behavioral break for anyone who relied on mutating the passed list and refreshing, so please classify it as breaking and note it in the changelog/release notes (e.g. "initial messages passed to the constructor are now defensively copied; mutate through sendMessage(...) instead of the supplied list").
There was a problem hiding this comment.
Acknowledged — kept intentional (defensive copy), not reverted. I tightened the constructor Javadoc to state that later changes to the supplied list are not reflected and that the conversation should be mutated via sendMessage(...) (commit 9de3af9). Will classify this as a breaking change in the release notes.
|
Full review of the current branch state (24b1b98), cross-checked against all prior review threads. Skipping items already raised in javier-godoy's reviews — these are additional findings. Blockers1. All frontend detach cleanup is dead code. 2. FAB drag/snap still ignores container bounds (raised twice by CodeRabbit, still open). 3. README sample doesn't compile. 4.
Should fix
NitsFormatter-mangled comments in |
Merge the redundant fabIcon/fabIconComponent fields into a single Component field, applying instanceof checks only where an SvgIcon is specifically required. Addresses PR #68 review r3545904693. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
setUnreadBadgeColors guarded the background with isBlank() but the text color with isEmpty(), so a whitespace-only color was applied verbatim instead of falling back to the default. Use isBlank() for both. Addresses PR #68 review r3546279320. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
updateMessage gated all content rewriting behind !isLoading(), so when the ComponentRenderer recycled a pooled VirtualList row to show a loading placeholder the previous message's text/markdown stayed visible next to the spinner. Always rewrite the content, blanking it while loading. Addresses PR #68 review r3546279308. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
addScreenSizeListener always observed the desktop popover overlay Div, but in MOBILE mode the chat content is moved into the mobile Dialog and the overlay is empty (0x0), so the listener never fired. Observe the container surface instead (it moves between both windows and a ResizeObserver follows it across DOM moves), and re-deliver the size on mobile dialog open for parity with the popover. Addresses PR #68 review r3546279298. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The default chatbot SVG was read in a static-final initializer, so a missing/unreadable resource threw at class-load time (ExceptionInInitializer Error, then NoClassDefFoundError on every later reference) even for callers supplying their own icon. Load it lazily on first use, cache the result, and fall back to no icon with a warning instead of throwing. Addresses PR #68 review r3546279303. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The client clamps window sizes numerically (parseFloat(value) + "px"), so a relative unit like "50%" or "30vw" passed to the String size setters was silently reinterpreted as pixels and clamped to the min bound. Validate the String width/height/min/max setters up front and fail with a clear message, matching the documented absolute-units contract. Addresses PR #68 review r3546279289. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Attaching a second ChatAssistant to a UI threw IllegalStateException, which could crash attach-before-detach route navigation or a dialog-over-view. Log a warning and leave the extra instance hidden and unwired instead, so it never competes with the already-attached instance and navigation is not broken. Addresses PR #68 review r3546279283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
movement.js disconnectedCallback removed the resize listener but never cleared the __fcChatAssistantMovement / fc-chat-assistant-drag-listener init guards, so after a detach/reattach addComponentRefreshedListener skipped re-initialization and the resize handler was never re-added (FAB could end up stranded off-screen). Clear both guards on disconnect. Addresses PR #68 review r3546279330. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
resize.js disconnectedCallback tore down the window resize listener, style observer and overlay-lookup timeout but never cleared the per-direction init guard on the durable resizer Div (nor the server refresh guard), so on reattach fcChatAssistantResize early-returned and the resize handlers/ indicators were never re-established. Clear both guards on disconnect (which only fires on a genuine detach, not on popover reopen). Addresses PR #68 review r3546279335. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The all-args builder constructor was public with many boxed Boolean flags that had to be null-defaulted in setUI. Make it private (construction is via ChatAssistant.builder() or the public legacy constructors) and switch the flags to primitive boolean so callers cannot pass null. A small partial ChatAssistantBuilder seeds the real defaults for the flags whose default is not false, and setUI no longer null-coalesces. Addresses PR #68 review r3545748934. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
applyGenericResizerStyle and setResizerClass are internal helpers used only within ChatAssistant to build/toggle the resize handles; they need not be part of the protected extension surface. Make them private. Addresses PR #68 review r3546022009. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
add/removeFabThemeVariants branched on variant.isSizeVariant() and then assumed the variant was LARGE or SMALL (variant == LARGE ? ... : SMALL). Inline the explicit SMALL/LARGE check so that assumption is provable at the call site and a future non-size variant cannot fall into the size branch. isSizeVariant() remains as public API. Addresses PR #68 review r3545985941. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Still open
|
|
@scardanzan thanks for the follow-up — addressed in these WIP commits:
|
|
@javier-godoy the three pending threads are answered inline:
On the review-level points:
|
|
@Totremont please do a squash of all the WIP commits and close this PR and create a new one so we can take a fresh look |
Swap the markdown-editor-addon MarkdownViewer for Vaadin's built-in Markdown component in ChatMessage.
Add the directional resize handles, window sizing with clamped min/max bounds applied to the popover content part, size persistence across close/reopen, the screen-size threshold tracking, and the resize direction indicators.
Replace the stale markdown-editor rule with rules that collapse the rendered markdown's outer block margins and tighten inter-block gaps.
Close #64 Add a Lombok @builder constructor and extend the component with: - FAB icon (default inline chatbot icon, custom overloads), sizing, and theme variants (color pass-through; LUMO_SMALL/LARGE drive the diameter). - FAB placement: setFabPosition/resetFabPosition, margin, setFabMovable, and setFabAnchoredToViewport for bounded placement. - Window control: setWindowResizable, setResizeIndicatorsVisible, initial size, and min/max bounds honored on open and while resizing; the window shrinks to its min height and clamps to the overlay. - Display mode: setMode/setMobileMode with a full-screen mobile dialog, breakpoint auto-switching (opt-in), and addModeChangedListener. - addScreenSizeListener for chat-window size threshold crossings.
Commented basic/lazy-loading/markdown/generative demos using the current API.
A FAB configuration demo (custom icon, size and color variants, section titles, movable/resizable/indicator toggles with notifications) and an in-a-box demo with a container-anchored FAB positioned via setFabPosition.
Desktop/mobile modes with breakpoint auto-switching, manual setMode, a mode-changed listener with notification, FAB position reset, and a chat window screen-size threshold listener.
Expand the feature list and replace the outdated getting-started example with current builder/setter-based tutorials covering messaging, FAB styling, window sizing, and responsive mobile mode.
WIP: address FAB API and behavior review feedback Introduce a FabVariant enum for FAB theming. Make isFabMovable() report the effective current state. Enforce a single ChatAssistant per UI. Use --lumo-warning-contrast-color as the badge text. Extract the --fc-min/max-* CSS property names into constants. Drop the duplicate setOpenOnClick(false). Add @SInCE 5.1.0 tags to the new public API. WIP: fix frontend listener cleanup and stale overlay references WIP: add unit tests for ChatAssistant pure-Java logic Cover parseFabMargin fallbacks, setUnreadMessages clamping to 0-99, addScreenSizeListener argument validation, and the FabVariant to ButtonVariant mapping, in the lightweight style of SerializationTest. WIP: add window sizing options to the builder Expose width, height, maxWidth and maxHeight on the ChatAssistant builder so the initial window size and its max bounds can be configured at construction time. WIP: apply Google Java Style WIP: normalize copyright year range WIP: support both Lumo and Aura themes Make the FAB color variants cross-theme: SUCCESS and ERROR now also carry an Aura accent CSS class (aura-accent-green/red) alongside the Lumo theme variant, since Aura styles accent colors via class rather than the theme attribute; PRIMARY stays on the cross-theme primary token and LUMO_CONTRAST is documented as Lumo-only. Replace the hardcoded --lumo-* tokens on the unread badge and in fc-chat-assistant-style.css (shadows, border radius, contrast) with cross-theme fallback chains (var(--lumo-x, var(--aura-y, <literal>))), mirroring the pattern already used in fc-chat-message-styles.css. WIP: report effective FAB drag state from isFabMovable isFabMovable() now returns fabMovable && fabAnchoredToViewport, matching the client drag gate (movable && anchored). A container-anchored FAB cannot be dragged, so the getter no longer reports true in that configuration; it already returned false in mobile mode. Adds a unit test covering the anchored/movable combinations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fix background-thread hygiene in demos ChatAssistantDemo replaces the leaked java.util.Timer (non-daemon, never cancelled) with a daemon-threaded ScheduledExecutorService that is shut down on detach. ChatAssistantGenerativeDemo runs its streaming task on a dedicated daemon executor instead of the shared ForkJoinPool.commonPool(), so its blocking sleeps no longer tie up common-pool threads; the executor is shut down on detach. Streaming behaviour and UI.access updates are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: simplify public javadoc, drop client-internal details Rewrite the javadoc on isFabMovable, setResizeIndicatorsVisible and the FabVariant enum so it describes behaviour from the caller's point of view rather than leaking client-side implementation details (the movable/anchored attributes, the Aura CSS-class-vs-theme-attribute mechanism, and Lumo design tokens). No behavioural change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: encapsulate FabVariant theming internals Make the ButtonVariant mapping and Aura CSS class private to FabVariant and expose intent-level applyColorTo(Button)/removeColorFrom(Button) instead of the toButtonVariant()/getAuraClass() accessors, so the underlying ButtonVariant and theme-class details no longer leak into the public API. ChatAssistant delegates color application to the variant; size handling is unchanged. Tests now verify the applied theme variant and Aura class through a real Button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: clarify only default functionality comply WIP: fix invalid CSS rejected by lightningcss (Vaadin 25.2) Check that the css follows the rules of lightningcss. Close #69 Close #67 WIP: make introduced variables private instead of protected WIP: store FAB icon once as a single Component field Merge the redundant fabIcon/fabIconComponent fields into a single Component field, applying instanceof checks only where an SvgIcon is specifically required. Addresses PR #68 review r3545904693. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reset unread badge text color on whitespace-only input setUnreadBadgeColors guarded the background with isBlank() but the text color with isEmpty(), so a whitespace-only color was applied verbatim instead of falling back to the default. Use isBlank() for both. Addresses PR #68 review r3546279320. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: clear recycled message row content while loading updateMessage gated all content rewriting behind !isLoading(), so when the ComponentRenderer recycled a pooled VirtualList row to show a loading placeholder the previous message's text/markdown stayed visible next to the spinner. Always rewrite the content, blanking it while loading. Addresses PR #68 review r3546279308. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make screen-size listener work in mobile mode addScreenSizeListener always observed the desktop popover overlay Div, but in MOBILE mode the chat content is moved into the mobile Dialog and the overlay is empty (0x0), so the listener never fired. Observe the container surface instead (it moves between both windows and a ResizeObserver follows it across DOM moves), and re-deliver the size on mobile dialog open for parity with the popover. Addresses PR #68 review r3546279298. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: load default FAB icon lazily and degrade gracefully The default chatbot SVG was read in a static-final initializer, so a missing/unreadable resource threw at class-load time (ExceptionInInitializer Error, then NoClassDefFoundError on every later reference) even for callers supplying their own icon. Load it lazily on first use, cache the result, and fall back to no icon with a warning instead of throwing. Addresses PR #68 review r3546279303. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reject non-pixel lengths in window-size setters The client clamps window sizes numerically (parseFloat(value) + "px"), so a relative unit like "50%" or "30vw" passed to the String size setters was silently reinterpreted as pixels and clamped to the min bound. Validate the String width/height/min/max setters up front and fail with a clear message, matching the documented absolute-units contract. Addresses PR #68 review r3546279289. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: keep duplicate ChatAssistant inert instead of throwing on attach Attaching a second ChatAssistant to a UI threw IllegalStateException, which could crash attach-before-detach route navigation or a dialog-over-view. Log a warning and leave the extra instance hidden and unwired instead, so it never competes with the already-attached instance and navigation is not broken. Addresses PR #68 review r3546279283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: re-init FAB movement after reattach movement.js disconnectedCallback removed the resize listener but never cleared the __fcChatAssistantMovement / fc-chat-assistant-drag-listener init guards, so after a detach/reattach addComponentRefreshedListener skipped re-initialization and the resize handler was never re-added (FAB could end up stranded off-screen). Clear both guards on disconnect. Addresses PR #68 review r3546279330. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: re-init resize handles after reattach resize.js disconnectedCallback tore down the window resize listener, style observer and overlay-lookup timeout but never cleared the per-direction init guard on the durable resizer Div (nor the server refresh guard), so on reattach fcChatAssistantResize early-returned and the resize handlers/ indicators were never re-established. Clear both guards on disconnect (which only fires on a genuine detach, not on popover reopen). Addresses PR #68 review r3546279335. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make builder constructor private and use primitive flags The all-args builder constructor was public with many boxed Boolean flags that had to be null-defaulted in setUI. Make it private (construction is via ChatAssistant.builder() or the public legacy constructors) and switch the flags to primitive boolean so callers cannot pass null. A small partial ChatAssistantBuilder seeds the real defaults for the flags whose default is not false, and setUI no longer null-coalesces. Addresses PR #68 review r3545748934. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make internal resizer styling helpers private applyGenericResizerStyle and setResizerClass are internal helpers used only within ChatAssistant to build/toggle the resize handles; they need not be part of the protected extension surface. Make them private. Addresses PR #68 review r3546022009. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: inline size-variant check in FAB theme variant methods add/removeFabThemeVariants branched on variant.isSizeVariant() and then assumed the variant was LARGE or SMALL (variant == LARGE ? ... : SMALL). Inline the explicit SMALL/LARGE check so that assumption is provable at the call site and a future non-size variant cannot fall into the size branch. isSizeVariant() remains as public API. Addresses PR #68 review r3545985941. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: use diamond operator in SerializationTest Drop the redundant explicit type argument on the right-hand side; the target type already provides it, per the style guide's diamond preference. Addresses PR #68 review r3546149078. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: merge duplicated popover/dialog content-part CSS rules The popover ::part(content) declaration block was duplicated across the Vaadin 24 (vaadin-popover-overlay) and Vaadin 25 (vaadin-popover) selectors, and likewise for the dialog pair. Combine each pair into a single comma-separated rule (valid under lightningcss) so future tweaks live in one place. Addresses PR #68 review r3546279367. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: share overlay resolution between resize.js helpers fetchOverlay and fcChatAssistantContentPart each hardcoded the same overlay selector chain (popover class -> shadowRoot -> vaadin-popover-overlay, with a getElementsByClassName fallback). Extract a single resolveOverlay(popoverTag) helper so the DOM structure is described once. Addresses PR #68 review r3546279356. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: deduplicate setFabIcon overloads Both setFabIcon(Component) and setFabIcon(Component, int) repeated the icon assignment, fab.setIcon and applyIconSize sequence. Extract a private applyFabIcon(icon, size) helper. The no-size overload keeps its own path (getFabIconSize() can be 0, which is valid here but rejected by the sized overload's size > 0 guard). Addresses PR #68 review r3546279361. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: defer window size/constraint push until the popover is open Each of the six window size/bound setters ran an executeJs that polled the popover content part up to 20x100ms; at construction the popover is closed, so a fully-configured builder spun ~6 pointless ~2s polling loops. Store the desired size in the --fc-* custom properties on the durable overlay Div from Java and only push to the client when the popover is open (the open listener already re-applies everything via fcChatAssistantRestoreWindowSize). The now-unused fcChatAssistantSetWindowSize JS helper is removed. Addresses PR #68 review r3546279350. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: hold resize handles in a direction-keyed map The eight resizer Divs were separate fields enumerated across three sites (edge styling, resizable class toggling, and client registration). Replace them with a single Map<String,Div> keyed by direction plus a RESIZER_DIRECTIONS list, and iterate that one collection at each site, so adding or renaming a handle is a single-line change. Per-direction edge styling moves into applyResizerEdgeStyle. Addresses PR #68 review r3546279344. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: run frontend teardown via an animated-fab custom element The root <animated-fab> element has no backing web component, so the root.disconnectedCallback assigned by the movement and resize modules was never invoked by the browser: the window resize listeners, style observer, overlay-lookup timeout and screen-size/mobile observers leaked on detach and the init guards stayed set (blocking reattach). Register a minimal animated-fab custom element whose disconnectedCallback runs teardown callbacks accumulated on root.__fcCleanups, and have both modules push their cleanup there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: delegate setFabIcon(Component) to the sized overload setFabIcon(Component) now calls setFabIcon(icon, getFabIconSize()) instead of duplicating the assignment/install/size logic. getFabIconSize() returns at least 1px so it never trips the sized overload's size > 0 guard when fabSize is small. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: don't nudge the FAB on a sub-threshold click pointermove mutated position before the sensitivity check returned, so a click with tiny movement was committed by stopDragging -> snapToBoundary -> updatePosition, shifting the FAB. Compute a candidate position and only commit it (and update the DOM) once the move exceeds the drag sensitivity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reject negative FAB margin parseFabMargin accepted a negative value (e.g. "-5"), which would pull the FAB off the edge. Fall back to the default for negatives, and cover it with a test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: abort generative demo streaming on interrupt streamWords swallowed InterruptedException (re-set the flag but kept emitting), so after onDetach's executor.shutdownNow() the loop kept calling ui.access on a detached view. Replace the stream with a plain loop whose Thread.sleep propagates the interrupt and aborts streaming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fix FAB theme-variant example in README addFabThemeVariants takes FabVariant, not ButtonVariant. Use FabVariant.LARGE/LUMO_CONTRAST in the snippet and correct the feature-list wording so the sample compiles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: correct @SInCE tags on window sizing/unread API Remove @SInCE 5.1.0 from the String window-size setters, getUnreadMessages and setUnreadMessages(int) (all present since 5.0.0), and add @SInCE 5.1.0 to setWindowResizable/isWindowResizable, which are new in 5.1.0 (also normalizing the malformed isWindowResizable javadoc). Verified against tag chat-assistant-addon-5.0.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: move FAB color application out of FabVariant FabVariant.applyColorTo/removeColorFrom were public methods that mutated a Button, leaking the apply logic. Make FabVariant a data-only enum exposing getButtonVariant()/getAuraClass(), and apply/remove the color from private ChatAssistant methods. Tests now assert the variant's data mapping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fill empty @PARAM in ChatMessage.updateMessage javadoc Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: trim redundant javadoc and reflow mangled comments Remove the Lombok-internals explanation on ChatAssistantBuilder, the breaking-change rationale on the mobile breakpoint, and the redundant single-instance comment in onAttach (the code and warning already say it). Reflow comments that had been wrapped into orphaned one-word lines (these render verbatim in the @demosource viewer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: drop write-only minWidth/minHeight fields The minWidth/minHeight fields were only ever assigned; the real state is the --fc-min-width/--fc-min-height custom properties on the overlay Div (read by fcChatAssistantApplyConstraints). Remove the dead fields and set the custom properties directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: target the animated-fab tag in integration tests The component root tag is animated-fab, but the ViewIT selector, the ChatAssistantElement page object and the demo CSS still referenced the old chat-bot tag. Update them so the ITs resolve the component (and, with the custom-element registration, ViewIT's upgrade assertion now holds). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: cover screen-size listener state in SerializationTest Register an addScreenSizeListener before serializing so the screen-size listener map (ScreenSizeListenerEntry) is exercised by the round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: simplify px concatenation and add a color literal fallback Drop the redundant String.valueOf(...) in the int window-size setters (int + "px" already yields a String), and add a literal fallback to the who-is-typing color var chain so it degrades when neither the Lumo nor Vaadin token is defined. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: assert animated-fab upgrade instead of shadow-root content Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: accept any CSS length for window size, drop px-only validation Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make internal FAB helpers private Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: place @SInCE before @param/@return per FC javadoc convention Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: document FAB movable no-op and defensive message copy Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the FAB being trapped by an ancestor that establishes a containing block for fixed descendants (e.g. Aura AppLayout navbar backdrop-filter): the anchored wrapper is lifted to document.body so position:fixed resolves against the viewport, and restored to its slot on detach. Close #70
0112ac8 to
d856400
Compare
|
github-project-automation
Bot
moved this from In Progress
to Done
in Flowing Code Addons
4 participants
Close #47 #64 #67
Chat Assistant Features (5.1.0-SNAPSHOT)
FAB (Floating Action Button)
Icon
setFabIcon(...), with a built-in chatbot icon used by default.Sizing
addFabThemeVariants(LUMO_SMALL, LUMO_LARGE)resizes the FAB (and its icon).LUMO_SUCCESS,LUMO_ERROR,LUMO_CONTRAST) restyle the FAB, with the icon automatically matching the button color.Positioning
setFabPosition(FabPosition)places the FAB in any of the four screen corners.resetFabPosition()restores the default position.Dragging
setFabMovable(...)allows the FAB to be dragged by the user.Bounded Placement
setFabAnchoredToViewport(false)positions the FAB inside a container instead of floating over the viewport.Unread badge colors
setUnreadBadgeColors(background, color)to change the badge background and text colors.Chat Window
Resizing
setWindowResizable(...)enables resizing using eight drag handles.setResizeIndicatorsVisible(...), showing arrows that indicate each handle's drag direction.Sizing & Bounds
setWindowWidth(...)setWindowHeight(...)setWindowMinWidth(...)setWindowMaxWidth(...)setWindowMinHeight(...)setWindowMaxHeight(...)Responsive / Mobile Mode
Modes
setMode(...)/setMobileMode(...)Auto-Switching
mobileBreakpoint.Listeners
addModeChangedListener(...)for mobile/desktop mode changes.addScreenSizeListener(...)for detecting when the chat window crosses a configured size threshold.Construction & Miscellaneous
Builder API
ChatAssistant.builder()provides a declarative configuration API.Native Markdown
markdown-editordependency has been removed.Summary by CodeRabbit
Summary
New Features
Bug Fixes
Documentation
Tests