Drop React FAB dependency by replacing ReactAdapter with vanilla component #58
Merged
scardanzan merged 4 commits intomasterfrom Apr 20, 2026
Merged
Drop React FAB dependency by replacing ReactAdapter with vanilla component #58scardanzan merged 4 commits intomasterfrom
scardanzan merged 4 commits intomasterfrom
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:
WalkthroughThis PR replaces the React-based FAB implementation with a pure Vaadin server-side component. It removes the React adapter, ReactAdapterComponent dependencies, and connector-based architecture, replacing them with vanilla JavaScript modules for dragging and resizing functionality. Updates include refactored state management, new CSS styling files, and revised demo applications using the updated FAB icon API. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 11
🧹 Nitpick comments (2)
src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css (1)
40-42: Consider specifying transition properties explicitly.Using
transition: allapplies to every CSS property change, which can impact performance. Specify the exact properties being animated (e.g.,opacity,transform,visibility) for better performance.⚡ Proposed optimization
.fc-chat-assistant-unread-badge { - transition: all 0.15s ease-out; + transition: opacity 0.15s ease-out, transform 0.15s ease-out; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css` around lines 40 - 42, The .fc-chat-assistant-unread-badge rule uses a broad "transition: all 0.15s ease-out;" which harms performance; replace it with explicit properties (for example "opacity, transform, visibility" or whichever properties the badge actually animates) and keep the same duration and timing (0.15s ease-out) so only those properties are transitioned; update the CSS selector .fc-chat-assistant-unread-badge accordingly.src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js (1)
20-101: A shared resize installer would pay for itself here.Only the edge predicate and delta math differ across the new
fc-chat-assistant-resize-*.jsfiles; the overlay lookup and pointer lifecycle are copied almost verbatim. That copy/paste surface has already drifted into a runtime bug in the bottom-left variant, so pulling the common pieces into one configurable helper would make future fixes land once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js` around lines 20 - 101, The current window.fcChatAssistantResizeBottomRight duplicates overlay lookup and pointer lifecycle logic (see fetchOverlay, overlay, isDragging and the pointerdown/pointermove/pointerup/pointerleave handlers) and should be refactored into a reusable installer: extract the common overlay fetching, shouldDrag check wiring, and pointer event lifecycle into a shared function (e.g., installFcChatResize) that accepts configurable callbacks or strategy functions for the edge predicate and delta math (the parts that differ across variants) and parameters size/maxSize; then replace window.fcChatAssistantResizeBottomRight with a thin adapter that passes its specific predicate and width/height delta calculations to installFcChatResize so fixes to overlay lookup or pointer handling apply to all variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 528-530: The setSubmitListener currently calls
this.defaultSubmitListenerRegistration.remove() and then adds a new listener,
but it never updates the stored Registration so subsequent calls remove the
originally-removed default instead of the previously-added custom listener;
change the logic to keep a dedicated Registration field (e.g.,
currentSubmitListenerRegistration) or reuse defaultSubmitListenerRegistration by
assigning it to the Registration returned from
messageInput.addSubmitListener(listener), and before adding check for null and
call remove() on the existing registration to properly detach the previous
custom listener; update the field to the new registration after adding.
- Around line 689-697: The badge UI uses the raw parameter instead of the
clamped field; update setUnreadMessages to use the clamped value
(this.unreadMessages) when calling unreadBadge.setText(...) and when deciding
visibility/scale so the displayed count and visibility match the stored capped
count (e.g., use this.unreadMessages for text and for the
unreadBadge.getStyle().setScale(...) condition inside the setUnreadMessages
method).
- Around line 365-375: The current dedup logic stores the marker in
document[uniqueFlag], which is global across all ChatAssistant instances; change
addComponentRefreshedListener so the marker is attached to the observed element
instead: in the executeJs payload use the element context (this) to check and
set the flag (e.g., this[flag] or this.dataset[flag]) rather than
document[flag], keeping the same then(...) behavior so callback.run() only
executes when the element's marker was not already set; update references to
uniqueFlag, root, and addComponentRefreshedListener accordingly.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 36-43: On window "resize" handler, don't reset
position.x/position.y to margin; instead update screenWidth and screenHeight
then clamp the existing position coordinates into the new viewport bounds using
margin (e.g., position.x = Math.max(margin, Math.min(position.x, screenWidth -
margin)) and similarly for position.y), then call updatePosition(); ensure you
reference the existing symbols screenWidth, screenHeight, position, margin, and
updatePosition() so the FAB keeps its chosen location but stays within the
resized viewport.
In
`@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-left.js`:
- Around line 59-70: Remove the stray assignment to newHeight inside the
pointerdown handler: delete the line that sets newHeight = offsetY +
container.clientHeight (offsetY is only defined in pointermove and causes a
ReferenceError during initial bottom-left drag). Keep the rest of the
pointerdown logic (setting isDragging, sizing the handle element, and
initializing minHeight/minWidth/maxHeight/maxWidth from container styles) and
let newHeight be computed later in the pointermove handler where offsetY is
available.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom.js`:
- Around line 56-83: The resize handle stops dragging when the pointer leaves
the handle because pointer events are only tracked while over item; update the
handlers to call event.target.setPointerCapture(e.pointerId) in the pointerdown
listener (after setting isDragging) and call
event.target.releasePointerCapture(e.pointerId) in pointerup/pointercancel,
remove or avoid treating pointerleave as cancel for active drags, and ensure
pointercancel also clears isDragging and resets item/container styles; apply the
same change to the listeners registered on item (pointerdown, pointerup,
pointercancel, pointerleave) so dragging continues outside the handle.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js`:
- Around line 42-43: The shouldDrag() boolean check currently inverts the
arrow-centered attribute by using getAttribute(overlayArrowCenteredAttribute) !=
""—replace that clause with hasAttribute(overlayArrowCenteredAttribute) so the
logic is true when the boolean attribute is present; update the same fix in the
shouldDrag (or equivalent) functions in fc-chat-assistant-resize-left.js
(function shouldDrag), fc-chat-assistant-resize-bottom-left.js, and
fc-chat-assistant-resize-top-left.js to use
hasAttribute(overlayArrowCenteredAttribute) instead of getAttribute(...) != "".
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js`:
- Around line 37-40: fetchOverlay currently finds an overlay by scanning
document.getElementsByClassName(popoverTag) which returns the first matching
popover globally; when multiple assistants exist this can pick the wrong overlay
and break shouldDrag. Change fetchOverlay to resolve the overlay relative to the
passed container/item (e.g., query within container or start from item.closest
or container.querySelector) using popoverTag/overlayTag to locate the correct
popover element, and ensure overlay is set to that scoped element so shouldDrag
reads the correct instance's top/right/bottom state.
In `@src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css`:
- Around line 44-48: Update the incorrect compass-direction comments for the
corner resize handle classes: change the comment on
.fc-chat-assistant-resize-bottom-left.active to "South-West",
.fc-chat-assistant-resize-bottom-right.active to "South-East",
.fc-chat-assistant-resize-top-left.active to "North-West", and
.fc-chat-assistant-resize-top-right.active to "North-East" so the inline
comments match the cursor directions (classes:
.fc-chat-assistant-resize-bottom-left.active,
.fc-chat-assistant-resize-bottom-right.active,
.fc-chat-assistant-resize-top-left.active,
.fc-chat-assistant-resize-top-right.active).
- Around line 50-54: The inline comments for the resize handle CSS are reversed:
update the comments on .fc-chat-assistant-resize-bottom.active and
.fc-chat-assistant-resize-top.active so they correctly identify directions
(bottom edge is South, top edge is North). Edit the comment after
.fc-chat-assistant-resize-bottom.active to read "South/Bottom" and the comment
after .fc-chat-assistant-resize-top.active to read "North/Top" to match the
cursor values.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java`:
- Line 107: The demo currently only adds chatAssistant to the layout, leaving
the previously created components message, chat, and chatWithThinking unused;
either remove their setup or re-add them to the layout so they are mounted and
exercised. Locate the ChatAssistantDemo class and either (A) remove the unused
setup code that constructs message, chat, and chatWithThinking, or (B) add those
components back alongside chatAssistant (e.g., via add(message, chat,
chatWithThinking, chatAssistant)) so the built and wired controls are attached
to the view and the demo exercises all paths.
---
Nitpick comments:
In
`@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js`:
- Around line 20-101: The current window.fcChatAssistantResizeBottomRight
duplicates overlay lookup and pointer lifecycle logic (see fetchOverlay,
overlay, isDragging and the pointerdown/pointermove/pointerup/pointerleave
handlers) and should be refactored into a reusable installer: extract the common
overlay fetching, shouldDrag check wiring, and pointer event lifecycle into a
shared function (e.g., installFcChatResize) that accepts configurable callbacks
or strategy functions for the edge predicate and delta math (the parts that
differ across variants) and parameters size/maxSize; then replace
window.fcChatAssistantResizeBottomRight with a thin adapter that passes its
specific predicate and width/height delta calculations to installFcChatResize so
fixes to overlay lookup or pointer handling apply to all variants.
In `@src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css`:
- Around line 40-42: The .fc-chat-assistant-unread-badge rule uses a broad
"transition: all 0.15s ease-out;" which harms performance; replace it with
explicit properties (for example "opacity, transform, visibility" or whichever
properties the badge actually animates) and keep the same duration and timing
(0.15s ease-out) so only those properties are transitioned; update the CSS
selector .fc-chat-assistant-unread-badge accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eee36596-aa1c-43d2-b37e-61f67908dbc7
⛔ 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 (19)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/resources/META-INF/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top.jssrc/main/resources/META-INF/frontend/react/animated-fab.tsxsrc/main/resources/META-INF/frontend/styles/chat-assistant-styles.csssrc/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/resources/frontend/fcChatAssistantConnector.jssrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.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.java
💤 Files with no reviewable changes (3)
- src/main/resources/META-INF/frontend/react/animated-fab.tsx
- src/main/resources/META-INF/resources/frontend/fcChatAssistantConnector.js
- src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js (1)
20-104: Extract a shared resize helper before these modules drift further.These directional files are already diverging in behavior and bug surface. For example,
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.jscurrently references an undeclaredoverlayArrowCenteredAttribute, while sibling files initialize bounds at different points (pointerentervspointerdown). A small shared helper with direction-specific callbacks would make fixes land once instead of eight times.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js` around lines 20 - 104, The resize logic in window.fcChatAssistantResizeBottomRight is duplicative and inconsistent with other directional modules (e.g., top-left) so extract a shared helper (e.g., createResizeHandle) that encapsulates overlay discovery (fetchOverlay), bounds initialization, pointer lifecycle (pointerenter, pointerdown, pointermove, pointerup, pointerleave), and shouldDrag logic, and accept direction-specific callbacks or configuration (initial size, which edges to compute offsets for, how to apply newWidth/newHeight). Replace the existing window.fcChatAssistantResizeBottomRight body with a thin wrapper that calls the shared helper, passing functions to compute offsets for bottom/right and to set container.style.width/height; also move common overlay polling (fetchOverlay) and pointer capture/release into the shared helper so fixes (like the missing overlayArrowCenteredAttribute and inconsistent init timing) are applied once across all direction modules.src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js (2)
30-31: Consider adding retry limit or cleanup for overlay discovery.The
fetchOverlayfunction is called viarequestAnimationFrameand a 2-secondsetTimeoutfallback. If the overlay never appears, the function silently fails without logging, which could make debugging difficult. Additionally, if the component is destroyed before the timeout fires, there's no cleanup mechanism.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js` around lines 30 - 31, The overlay discovery calls currently schedule fetchOverlay via window.requestAnimationFrame and a fallback setTimeout without a retry limit or cleanup; update the logic in fc-chat-assistant-resize-left.js to add a bounded retry mechanism (e.g., retry count or maxElapsed time) when invoking fetchOverlay, log failures or final timeout so missing overlays are visible, and store/clear the handles so you can cancel them (clearTimeout and cancelAnimationFrame) if the component is destroyed or the overlay is found; specifically modify the code around the requestAnimationFrame/setTimeout calls and any component teardown or destroy path to cancel outstanding timers and to increment/inspect the retry counter before scheduling another fetchOverlay.
45-53: Pointer capture onpointerenteris unconventional and may cause issues.Typically,
setPointerCaptureis called onpointerdown, notpointerenter. Capturing on enter means the element captures all pointer events as soon as the cursor hovers over it, which could interfere with other interactions on the page.♻️ Suggested fix: move capture to pointerdown
item.addEventListener('pointerenter', (e) => { if (shouldDrag()) { item.classList.add('active'); - item.setPointerCapture(e.pointerId); } else { item.classList.remove('active'); } }); item.addEventListener('pointerdown', (_) => { + item.addEventListener('pointerdown', (e) => { isDragging = shouldDrag(); if (isDragging) { + item.setPointerCapture(e.pointerId); item.style.width = maxSize + 'px';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js` around lines 45 - 53, The pointer capture is being set inside the pointerenter handler which is incorrect; remove setPointerCapture from the pointerenter listener on item and instead add a pointerdown listener on item that calls item.setPointerCapture(e.pointerId) when shouldDrag() returns true and adds the 'active' class (mirror the existing logic), and ensure you release capture in pointerup and pointercancel handlers using item.releasePointerCapture(e.pointerId) and remove the 'active' class there; keep pointerenter only for UI hover state (class toggling) and reference the existing item, shouldDrag(), setPointerCapture, pointerdown, pointerup, and releasePointerCapture symbols when making the change.src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js (1)
20-28: FAB dimensions captured once at initialization.The
itemWidthanditemHeightare captured fromfab.clientWidth/clientHeightat function setup time (lines 21-22). If the FAB size changes after initialization (e.g., via CSS or programmatic resize), the boundary calculations insnapToBoundarywould use stale values. Given the current fixedDEFAULT_FAB_SIZEin Java, this is low risk but worth noting for future flexibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js` around lines 20 - 28, The code captures fab dimensions once (itemWidth/itemHeight) in window.fcChatAssistantMovement which can become stale if the FAB is resized; update the logic so boundary math uses current sizes — either read fab.clientWidth and fab.clientHeight inside snapToBoundary (or any function that computes boundaries) instead of relying on the initial itemWidth/itemHeight, or attach a ResizeObserver on fab to update itemWidth/itemHeight and re-run boundary/snap logic (references: window.fcChatAssistantMovement, itemWidth, itemHeight, snapToBoundary, fab).src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (4)
334-342: Consider potential race condition with rapid clicks during popover transition.The
onClick()method toggles the chat window based onchatWindow.isOpened(). If rapid clicks occur before the popover completes its open/close transition, server state and DOM state could temporarily diverge. Vaadin's server-authoritative model should eventually sync, but a brief client-side debounce infc-chat-assistant-movement.jscould provide smoother UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 334 - 342, The onClick() handler toggles chatWindow based on chatWindow.isOpened(), which can race with rapid user clicks during the popover open/close transition; add a short client-side debounce or transition lock in fc-chat-assistant-movement.js (e.g., track an inTransition flag or disable the trigger for ~200–300ms) so additional clicks are ignored while the popover is animating, and keep the server call (ClientCallable onClick) unchanged so Vaadin remains authoritative; reference the onClick method, chatWindow.isOpened(), and fc-chat-assistant-movement.js when implementing the debounce/lock and ensure you clear the flag after the transition completes.
362-373: Dedup logic fix looks correct, but the stored value should be a boolean.The fix now uses
this[flag]instead ofdocument[flag], which addresses the previous concern about global state. However, line 367 assignsthis[flag] = thisinstead of a boolean, which is unconventional. While truthy, storing the element reference is unnecessary.♻️ Minor refinement
const flag = $0; if(this[flag]) return true; - this[flag] = this; + this[flag] = true; return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 362 - 373, In addComponentRefreshedListener, the JS currently stores the element reference with this[flag] = this; change it to store a boolean instead so the dedup flag is explicit: set this[flag] = true when marking that the callback has run, keep the initial check if (this[flag]) return true; and preserve the Promise handling that calls callback.run() when the JS returns false; this uses the uniqueFlag parameter and callback Runnable already present.
533-536: Remove unused private methodrefreshContent.This method is never called and duplicates the logic already present in
sendMessage(lines 545-548).♻️ Remove unused method
- private void refreshContent() { - this.content.getDataProvider().refreshAll(); - this.content.scrollToEnd(); - } -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 533 - 536, Remove the unused private method refreshContent() from the ChatAssistant class: delete the method block for refreshContent() (which calls this.content.getDataProvider().refreshAll() and this.content.scrollToEnd()) since its logic is already duplicated in sendMessage(), ensure no remaining references to refreshContent() exist, and run a build to confirm there are no compilation issues after removal.
115-115: Add type parameter to avoid raw type usage.The
VirtualListinstantiation should include the generic type parameter to maintain type safety.♻️ Fix raw types
- this.content = new VirtualList(); + this.content = new VirtualList<>();Similarly for lines 124 and 128:
- this(new ArrayList(), false); + this(new ArrayList<>(), false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` at line 115, The VirtualList instantiations in class ChatAssistant are using raw types; update the new VirtualList() calls that assign to the field content (and the other two similar instantiations) to include the appropriate generic type parameter matching the declared type of content (for example new VirtualList<Component>() or new VirtualList<Message>() based on your content field type) so the code no longer uses raw types and retains compile-time type safety.src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.js (1)
20-104: Consider extracting shared resize logic into a common utility.This file shares nearly identical structure with
fc-chat-assistant-resize-left.jsand other resize utilities (overlay discovery, pointer event handling, drag state management). Extracting common logic into a shared module would reduce duplication and simplify maintenance.The same issues noted for
fc-chat-assistant-resize-left.jsapply here:
- Pointer capture on
pointerenterinstead ofpointerdown- Unguarded
releasePointerCapturecalls🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.js` around lines 20 - 104, Extract duplicate overlay discovery and pointer-drag handling into a shared utility module used by window.fcChatAssistantResizeTopRight (and fc-chat-assistant-resize-left.js): move the fetchOverlay and shouldDrag logic plus common pointer event handlers into reusable functions so both files call the same implementation; then fix event handling in window.fcChatAssistantResizeTopRight by moving pointer capture from the pointerenter handler to pointerdown (call item.setPointerCapture(e.pointerId) inside pointerdown using the event argument) and guard pointer release calls in pointerup and pointerleave (only call item.releasePointerCapture(e.pointerId) if item.hasPointerCapture(e.pointerId) is true), keeping unique resize-specific logic (size, maxSize, min/max calculations and container.style updates) in fcChatAssistantResizeTopRight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 243-244: The badge text is inconsistently initialized to "99" in
setUI() while the unreadMessages field defaults to 0; fix by making the initial
badge state match the model: either remove the hardcoded
unreadBadge.setText("99") call in setUI() or replace it by invoking
setUnreadMessages(0) so unreadMessages and the displayed badge are aligned;
target the unreadBadge initialization in setUI() and the
setUnreadMessages/getUnreadMessages fields to ensure consistent initial state.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 122-124: The call to root.$server.onClick() can throw if the
server reference is missing; update the isClickOnlyEvent() handling to first
verify the server exists and exposes an onClick method (e.g., check root.$server
and typeof root.$server.onClick === 'function') before invoking it so the code
only calls onClick when the server reference is present and callable.
- Around line 57-65: The current adjustment can set container.style.width/height
to 0px when widthAdjustment/heightAdjustment exceed rect.width/rect.height;
update the logic in the block that uses widthAdjustment, heightAdjustment, rect,
and container.style.width/height to enforce minimum dimensions (e.g., define
MIN_WIDTH and MIN_HEIGHT constants or read container's computed
min-width/min-height) and use Math.max(MIN_WIDTH, rect.width - widthAdjustment)
and Math.max(MIN_HEIGHT, rect.height - heightAdjustment) so the container never
shrinks below the minimum.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js`:
- Around line 74-86: The pointerup/pointerleave handlers call
item.releasePointerCapture(e.pointerId) unconditionally which can throw if the
element does not have capture; modify both handlers (the anonymous functions
attached via item.addEventListener('pointerup', ...) and
item.addEventListener('pointerleave', ...)) to guard the release by checking
item.hasPointerCapture(e.pointerId) before calling releasePointerCapture (or
wrap the call in a try/catch and swallow InvalidStateError) so
releasePointerCapture is only invoked when safe.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js`:
- Around line 43-46: The function shouldDrag() uses
overlayArrowCenteredAttribute which is not declared; declare and initialize
overlayArrowCenteredAttribute (e.g., const overlayArrowCenteredAttribute =
'data-overlay-arrow-centered' or the correct attribute name used elsewhere)
before shouldDrag() is defined so that shouldDrag(), and the
pointerenter/pointerdown handlers, can safely call
overlay.hasAttribute(overlayArrowCenteredAttribute) without causing a
ReferenceError; ensure the identifier name matches other code that sets or reads
that attribute.
In `@src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css`:
- Line 1: The license banner's opener `/*-` triggers the Stylelint rule
`comment-whitespace-inside`; fix by either changing the opener to a standard
block comment `/*` (replace `/*-` with `/*`) or exempting the banner from
Stylelint by adding a top-of-file disable/enable pair around the banner (e.g.,
`/* stylelint-disable comment-whitespace-inside */` before the banner and `/*
stylelint-enable comment-whitespace-inside */` after) so
`comment-whitespace-inside` no longer fails.
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 334-342: The onClick() handler toggles chatWindow based on
chatWindow.isOpened(), which can race with rapid user clicks during the popover
open/close transition; add a short client-side debounce or transition lock in
fc-chat-assistant-movement.js (e.g., track an inTransition flag or disable the
trigger for ~200–300ms) so additional clicks are ignored while the popover is
animating, and keep the server call (ClientCallable onClick) unchanged so Vaadin
remains authoritative; reference the onClick method, chatWindow.isOpened(), and
fc-chat-assistant-movement.js when implementing the debounce/lock and ensure you
clear the flag after the transition completes.
- Around line 362-373: In addComponentRefreshedListener, the JS currently stores
the element reference with this[flag] = this; change it to store a boolean
instead so the dedup flag is explicit: set this[flag] = true when marking that
the callback has run, keep the initial check if (this[flag]) return true; and
preserve the Promise handling that calls callback.run() when the JS returns
false; this uses the uniqueFlag parameter and callback Runnable already present.
- Around line 533-536: Remove the unused private method refreshContent() from
the ChatAssistant class: delete the method block for refreshContent() (which
calls this.content.getDataProvider().refreshAll() and
this.content.scrollToEnd()) since its logic is already duplicated in
sendMessage(), ensure no remaining references to refreshContent() exist, and run
a build to confirm there are no compilation issues after removal.
- Line 115: The VirtualList instantiations in class ChatAssistant are using raw
types; update the new VirtualList() calls that assign to the field content (and
the other two similar instantiations) to include the appropriate generic type
parameter matching the declared type of content (for example new
VirtualList<Component>() or new VirtualList<Message>() based on your content
field type) so the code no longer uses raw types and retains compile-time type
safety.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 20-28: The code captures fab dimensions once
(itemWidth/itemHeight) in window.fcChatAssistantMovement which can become stale
if the FAB is resized; update the logic so boundary math uses current sizes —
either read fab.clientWidth and fab.clientHeight inside snapToBoundary (or any
function that computes boundaries) instead of relying on the initial
itemWidth/itemHeight, or attach a ResizeObserver on fab to update
itemWidth/itemHeight and re-run boundary/snap logic (references:
window.fcChatAssistantMovement, itemWidth, itemHeight, snapToBoundary, fab).
In
`@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js`:
- Around line 20-104: The resize logic in
window.fcChatAssistantResizeBottomRight is duplicative and inconsistent with
other directional modules (e.g., top-left) so extract a shared helper (e.g.,
createResizeHandle) that encapsulates overlay discovery (fetchOverlay), bounds
initialization, pointer lifecycle (pointerenter, pointerdown, pointermove,
pointerup, pointerleave), and shouldDrag logic, and accept direction-specific
callbacks or configuration (initial size, which edges to compute offsets for,
how to apply newWidth/newHeight). Replace the existing
window.fcChatAssistantResizeBottomRight body with a thin wrapper that calls the
shared helper, passing functions to compute offsets for bottom/right and to set
container.style.width/height; also move common overlay polling (fetchOverlay)
and pointer capture/release into the shared helper so fixes (like the missing
overlayArrowCenteredAttribute and inconsistent init timing) are applied once
across all direction modules.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js`:
- Around line 30-31: The overlay discovery calls currently schedule fetchOverlay
via window.requestAnimationFrame and a fallback setTimeout without a retry limit
or cleanup; update the logic in fc-chat-assistant-resize-left.js to add a
bounded retry mechanism (e.g., retry count or maxElapsed time) when invoking
fetchOverlay, log failures or final timeout so missing overlays are visible, and
store/clear the handles so you can cancel them (clearTimeout and
cancelAnimationFrame) if the component is destroyed or the overlay is found;
specifically modify the code around the requestAnimationFrame/setTimeout calls
and any component teardown or destroy path to cancel outstanding timers and to
increment/inspect the retry counter before scheduling another fetchOverlay.
- Around line 45-53: The pointer capture is being set inside the pointerenter
handler which is incorrect; remove setPointerCapture from the pointerenter
listener on item and instead add a pointerdown listener on item that calls
item.setPointerCapture(e.pointerId) when shouldDrag() returns true and adds the
'active' class (mirror the existing logic), and ensure you release capture in
pointerup and pointercancel handlers using
item.releasePointerCapture(e.pointerId) and remove the 'active' class there;
keep pointerenter only for UI hover state (class toggling) and reference the
existing item, shouldDrag(), setPointerCapture, pointerdown, pointerup, and
releasePointerCapture symbols when making the change.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.js`:
- Around line 20-104: Extract duplicate overlay discovery and pointer-drag
handling into a shared utility module used by
window.fcChatAssistantResizeTopRight (and fc-chat-assistant-resize-left.js):
move the fetchOverlay and shouldDrag logic plus common pointer event handlers
into reusable functions so both files call the same implementation; then fix
event handling in window.fcChatAssistantResizeTopRight by moving pointer capture
from the pointerenter handler to pointerdown (call
item.setPointerCapture(e.pointerId) inside pointerdown using the event argument)
and guard pointer release calls in pointerup and pointerleave (only call
item.releasePointerCapture(e.pointerId) if item.hasPointerCapture(e.pointerId)
is true), keeping unique resize-specific logic (size, maxSize, min/max
calculations and container.style updates) in fcChatAssistantResizeTopRight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebd57c4f-4482-49fd-8279-9edf7c19f6b3
📒 Files selected for processing (14)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.javasrc/main/resources/META-INF/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top.jssrc/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/frontend/styles/fc-chat-message-styles.csssrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js (1)
20-111: Consider a shared resizer factory for the eight directional helpers.This file repeats the same overlay lookup, pointer lifecycle, and cleanup pattern as
fc-chat-assistant-resize-top.js,...right.js,...bottom.js, etc. Only the gating predicate and axis math change. Pulling the common logic into one helper would make fixes land once instead of eight times.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js` around lines 20 - 111, The resize helper window.fcChatAssistantResizeTopLeft duplicates overlay lookup and pointer lifecycle logic found in the other directional files; refactor by extracting a shared resizer factory (e.g., createPopoverResizer) that encapsulates fetchOverlay, overlay caching, pointer event wiring (pointerenter/down/move/up/leave), pointer capture/release, min/max parsing and cleanup, and accepts direction-specific callbacks or a small strategy object (e.g., shouldDrag predicate and computeNewSize functions for X/Y axes) used by fcChatAssistantResizeTopLeft; replace the duplicated logic in window.fcChatAssistantResizeTopLeft with a call to the factory passing a shouldDrag implementation and axis math for top-left resizing, and update the other fc-chat-assistant-resize-*.js files to use the same factory so fixes are centralized.src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (3)
113-130: Use parameterized types instead of raw generics.Raw generic types (
new VirtualList(),new ArrayList()) generate compiler warnings and bypass type safety. Use the diamond operator for type inference.♻️ Suggested fix
public ChatAssistant(List<T> messages, boolean markdownEnabled) { this.setUI(); - this.content = new VirtualList(); + this.content = new VirtualList<>(); this.messages = messages; this.initializeHeader(); this.initializeFooter(); this.initializeContent(markdownEnabled); this.initializeChatWindow(); } public ChatAssistant() { - this(new ArrayList(), false); + this(new ArrayList<>(), false); } public ChatAssistant(boolean markdownEnabled) { - this(new ArrayList(), markdownEnabled); + this(new ArrayList<>(), markdownEnabled); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 113 - 130, The constructors in ChatAssistant use raw generic instantiations (new VirtualList(), new ArrayList()) causing unsafe types; update the constructors (the ChatAssistant(List<T> messages, boolean markdownEnabled), ChatAssistant() and ChatAssistant(boolean)) to instantiate with parameterized/dia mond types matching T and the VirtualList element type (e.g., new ArrayList<>(), new VirtualList<>()) so the class is type-safe and compiler warnings are removed.
426-426: Remove unnecessary array creation in varargs calls.Vaadin's layout methods accept varargs, so explicit array creation is unnecessary. This applies to multiple locations in the file (lines 426, 443, 465, 574, 598, 600).
♻️ Example fix for line 426
- HorizontalLayout header = new HorizontalLayout(new Component[]{title, minimize}); + HorizontalLayout header = new HorizontalLayout(title, minimize);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` at line 426, In ChatAssistant, remove unnecessary explicit array creation when calling Vaadin vararg constructors; replace expressions like new HorizontalLayout(new Component[]{title, minimize}) with new HorizontalLayout(title, minimize) (and similarly for other instances such as the HorizontalLayout/VerticalLayout constructions found elsewhere in the file), ensuring you pass the Component instances directly to the varargs constructor rather than creating a Component[].
238-250: Extract duplicate CSS literal to a constant.The literal
"var(--lumo-font-size-xs)"is used 7 times for the unread badge sizing. Consider extracting it to a constant for maintainability.♻️ Suggested fix
+ private static final String BADGE_SIZE = "var(--lumo-font-size-xs)"; // ... unreadBadge.getStyle() // ... - .setFontSize("var(--lumo-font-size-xs)") + .setFontSize(BADGE_SIZE) // ... - .setMinHeight("var(--lumo-font-size-xs)") - .setMinWidth("var(--lumo-font-size-xs)") - .setHeight("var(--lumo-font-size-xs)") - .setWidth("var(--lumo-font-size-xs)") - .setMaxHeight("var(--lumo-font-size-xs)") - .setMaxWidth("var(--lumo-font-size-xs)") + .setMinHeight(BADGE_SIZE) + .setMinWidth(BADGE_SIZE) + .setHeight(BADGE_SIZE) + .setWidth(BADGE_SIZE) + .setMaxHeight(BADGE_SIZE) + .setMaxWidth(BADGE_SIZE)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 238 - 250, The repeated literal "var(--lumo-font-size-xs)" in the unread badge styling should be extracted to a private static final constant (e.g., SMALL_FONT_SIZE or UNREAD_BADGE_SIZE) in the ChatAssistant class and replace all seven occurrences in the badge builder chain (.setFontSize, .setMinHeight, .setMinWidth, .setHeight, .setWidth, .setMaxHeight, .setMaxWidth) with that constant; update the badge creation code to reference the constant to improve maintainability.src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js (2)
52-56: Setting pointer capture onpointerentermay cause unexpected capture behavior.Calling
setPointerCapture(e.pointerId)in thepointerenterhandler means the element captures the pointer as soon as the cursor enters, even before the user intends to drag. This can interfere with other interactive elements and cause the pointer to remain captured even when the user hovers without clicking.Consider moving
setPointerCaptureto thepointerdownhandler instead, which aligns with standard drag interaction patterns.♻️ Suggested refactor
item.addEventListener('pointerenter', (e) => { if (shouldDrag()) { item.classList.add('active'); - item.setPointerCapture(e.pointerId); } else { item.classList.remove('active'); } }); item.addEventListener('pointerdown', (_) => { + item.addEventListener('pointerdown', (e) => { isDragging = shouldDrag(); if (isDragging) { + item.setPointerCapture(e.pointerId); item.style.height = maxSize + 'px';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js` around lines 52 - 56, The pointer capture is being set in the pointerenter handler which causes premature capture; update the logic so that item.setPointerCapture(e.pointerId) is removed from the 'pointerenter' listener and instead invoked inside the 'pointerdown' handler when shouldDrag() is true and a drag is actually starting. Keep the existing item.classList.add('active') behavior in the 'pointerenter' handler if you still want hover styling, and ensure the 'pointerdown' handler (the function attached to item.addEventListener('pointerdown', ...)) calls setPointerCapture(e.pointerId) and handles subsequent pointermove/pointerup cleanup.
90-110: Duplicate cleanup logic inpointerupandpointerleavehandlers.Both handlers contain identical reset logic. Consider extracting to a shared helper function for maintainability.
♻️ Suggested refactor
+ function resetDragState(e) { + isDragging = false; + item.style.height = size + 'px'; + item.style.width = size + 'px'; + item.style.marginBottom = ''; + item.style.marginRight = ''; + if (item.hasPointerCapture(e.pointerId)) { + item.releasePointerCapture(e.pointerId); + } + } + - item.addEventListener('pointerup', (e) => { - isDragging = false; - item.style.height = size + 'px'; - item.style.width = size + 'px'; - item.style.marginBottom = ''; - item.style.marginRight = ''; - if (item.hasPointerCapture(e.pointerId)) { - item.releasePointerCapture(e.pointerId); - } - }); - - item.addEventListener('pointerleave', (e) => { - isDragging = false; - item.style.height = size + 'px'; - item.style.width = size + 'px'; - item.style.marginBottom = ''; - item.style.marginRight = ''; - if (item.hasPointerCapture(e.pointerId)) { - item.releasePointerCapture(e.pointerId); - } - }); + item.addEventListener('pointerup', resetDragState); + item.addEventListener('pointerleave', resetDragState);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js` around lines 90 - 110, Both pointerup and pointerleave handlers contain duplicated cleanup logic; extract that logic into a shared helper function (e.g., resetResizeState or cleanupDrag) and call it from both item.addEventListener('pointerup', ...) and item.addEventListener('pointerleave', ...). The helper should set isDragging = false, reset item.style.height/width to size + 'px', clear item.style.marginBottom and item.style.marginRight, and release pointer capture if item.hasPointerCapture(e.pointerId) (pass the event or pointerId into the helper so it can call item.releasePointerCapture). Update the two event listeners to simply call this helper to remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 201-203: The overlay currently only sets minHeight/minWidth (in
the block using overlay.getStyle() and constants
DEFAULT_CONTENT_MIN_HEIGHT/DEFAULT_CONTENT_MIN_WIDTH), but the resize script
fc-chat-assistant-resize-bottom-right.js (function shouldDrag()) expects inline
positioning styles; update the overlay style initialization to also set explicit
positioning properties (e.g., top/left or bottom/right) via
overlay.getStyle().set("top", "...px")/set("left", "...px") (or equivalent API
used elsewhere in ChatAssistant) so the resize detection finds inline values, or
alternatively adjust the JS shouldDrag() to check computedStyle as a fallback —
pick the first option (set inline top/left or bottom/right on overlay) and
ensure the same positioning keys used by shouldDrag() are set.
- Around line 348-359: The deduplication condition in
addComponentRefreshedListener is inverted so the callback keeps running because
the flag is never set; modify the JS emitted for uniqueFlag and executable so
that after executing the callback (%2$s) you set this['%1$s'] = this (i.e., move
or add the flag assignment into the if branch immediately after running
executable) so subsequent calls see the flag and skip re-execution.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 101-129: Add a pointercancel listener on the same element (item)
to mirror the pointerup cleanup: when a pointercancel occurs set isDragging =
false, remove the 'dragging' class from fab, set item.style.transition to the
same snapTransition + ', ' + sizeTransition used in pointerup, call
item.releasePointerCapture(e.pointerId) if captured, and call snapToBoundary()
to reset the FAB position; do NOT invoke root.$server?.onClick() or any
click-only logic in this handler. Reference the existing
pointerdown/pointermove/pointerup handlers and the isDragging, fab,
item.releasePointerCapture, snapTransition, sizeTransition, snapToBoundary, and
isClickOnlyEvent symbols when adding the pointercancel handler.
- Around line 34-131: The movement initializer (window.fcChatAssistantMovement)
registers resize, pointerdown, pointermove and pointerup handlers every time it
runs, causing listener duplication; add an idempotence guard by checking/setting
a single boolean flag (e.g., window.fcChatAssistantMovementInitialized) at the
top of the initializer and return early if already true, and set the flag to
true immediately after registering the listeners so subsequent calls skip
registering; keep existing functions (updatePosition, snapToBoundary,
isClickOnlyEvent) and listener registration code intact while ensuring the guard
prevents re-registration.
In
`@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js`:
- Around line 46-50: The shouldDrag() function currently inspects
overlay.style.top/bottom/left/right which are never set (ChatAssistant.java only
sets minHeight/minWidth), so it always returns false; update shouldDrag() to use
window.getComputedStyle(overlay) to read computed top/right/bottom/left (or
otherwise detect positioning via computed position or a CSS class) and base the
bottomRule/rightRule on those computed values (referencing the shouldDrag
function and overlay element) so the resize logic enables correctly; apply the
same change pattern to the other seven resize utility files that use the same
inline-style checks.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top.js`:
- Around line 48-93: Create a shared stopDragging() helper and use it from the
existing pointerup and pointerleave handlers and add a new pointercancel
listener; stopDragging() should set isDragging = false, reset item.style.height
= size + 'px' and item.style.marginTop = '', remove the 'active' class from
item, and if item.hasPointerCapture(e.pointerId) release it (accepting the event
or pointerId as needed), then replace the duplicated reset logic in
pointerup/pointerleave with a call to stopDragging() and add
item.addEventListener('pointercancel', stopDragging) so canceled gestures clear
the sticky hover state and stale isDragging flag.
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 113-130: The constructors in ChatAssistant use raw generic
instantiations (new VirtualList(), new ArrayList()) causing unsafe types; update
the constructors (the ChatAssistant(List<T> messages, boolean markdownEnabled),
ChatAssistant() and ChatAssistant(boolean)) to instantiate with
parameterized/dia mond types matching T and the VirtualList element type (e.g.,
new ArrayList<>(), new VirtualList<>()) so the class is type-safe and compiler
warnings are removed.
- Line 426: In ChatAssistant, remove unnecessary explicit array creation when
calling Vaadin vararg constructors; replace expressions like new
HorizontalLayout(new Component[]{title, minimize}) with new
HorizontalLayout(title, minimize) (and similarly for other instances such as the
HorizontalLayout/VerticalLayout constructions found elsewhere in the file),
ensuring you pass the Component instances directly to the varargs constructor
rather than creating a Component[].
- Around line 238-250: The repeated literal "var(--lumo-font-size-xs)" in the
unread badge styling should be extracted to a private static final constant
(e.g., SMALL_FONT_SIZE or UNREAD_BADGE_SIZE) in the ChatAssistant class and
replace all seven occurrences in the badge builder chain (.setFontSize,
.setMinHeight, .setMinWidth, .setHeight, .setWidth, .setMaxHeight, .setMaxWidth)
with that constant; update the badge creation code to reference the constant to
improve maintainability.
In
`@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js`:
- Around line 52-56: The pointer capture is being set in the pointerenter
handler which causes premature capture; update the logic so that
item.setPointerCapture(e.pointerId) is removed from the 'pointerenter' listener
and instead invoked inside the 'pointerdown' handler when shouldDrag() is true
and a drag is actually starting. Keep the existing item.classList.add('active')
behavior in the 'pointerenter' handler if you still want hover styling, and
ensure the 'pointerdown' handler (the function attached to
item.addEventListener('pointerdown', ...)) calls setPointerCapture(e.pointerId)
and handles subsequent pointermove/pointerup cleanup.
- Around line 90-110: Both pointerup and pointerleave handlers contain
duplicated cleanup logic; extract that logic into a shared helper function
(e.g., resetResizeState or cleanupDrag) and call it from both
item.addEventListener('pointerup', ...) and
item.addEventListener('pointerleave', ...). The helper should set isDragging =
false, reset item.style.height/width to size + 'px', clear
item.style.marginBottom and item.style.marginRight, and release pointer capture
if item.hasPointerCapture(e.pointerId) (pass the event or pointerId into the
helper so it can call item.releasePointerCapture). Update the two event
listeners to simply call this helper to remove duplication.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js`:
- Around line 20-111: The resize helper window.fcChatAssistantResizeTopLeft
duplicates overlay lookup and pointer lifecycle logic found in the other
directional files; refactor by extracting a shared resizer factory (e.g.,
createPopoverResizer) that encapsulates fetchOverlay, overlay caching, pointer
event wiring (pointerenter/down/move/up/leave), pointer capture/release, min/max
parsing and cleanup, and accepts direction-specific callbacks or a small
strategy object (e.g., shouldDrag predicate and computeNewSize functions for X/Y
axes) used by fcChatAssistantResizeTopLeft; replace the duplicated logic in
window.fcChatAssistantResizeTopLeft with a call to the factory passing a
shouldDrag implementation and axis math for top-left resizing, and update the
other fc-chat-assistant-resize-*.js files to use the same factory so fixes are
centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ff9021e-43ae-49f8-af34-48eb7c87c9f2
📒 Files selected for processing (13)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/resources/META-INF/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-right.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize-top.jssrc/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/frontend/styles/fc-chat-message-styles.css
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/META-INF/frontend/styles/fc-chat-message-styles.css
🚧 Files skipped from review as they are similar to previous changes (2)
- pom.xml
- src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/resources/META-INF/frontend/fc-chat-assistant-resize.js (2)
34-36: Use optional chaining for cleaner null-safe property access.The
shouldDragfunctions can benefit from optional chaining to improve readability and avoid potential issues ifoverlayis undefined.♻️ Suggested refactor for the 'top' direction
'top': { - shouldDrag: () => overlay && overlay.style.bottom && !overlay.style.top, + shouldDrag: () => overlay?.style.bottom && !overlay?.style.top,The same pattern applies to other
shouldDragfunctions at lines 55-56, 85, 104-105, 134, 153-154, 183, and 202-203.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize.js` around lines 34 - 36, Several shouldDrag functions inside the directionConfig object (e.g., the 'top' entry's shouldDrag and the other shouldDrag functions referenced) access overlay.style properties without null-safety; change these to use optional chaining when reading overlay and overlay.style (e.g., overlay?.style?.bottom and overlay?.style?.top) so the checks are safe if overlay is undefined, and apply the same pattern to all shouldDrag definitions in directionConfig to avoid runtime errors.
22-31: Consider adding a client-side idempotence guard.While the Java-side
addComponentRefreshedListenerprovides protection against duplicate initialization, adding a JS-side guard would be safer defensive coding—especially if this function is ever called directly or if the Java guard logic changes.🛡️ Suggested defensive guard
window.fcChatAssistantResize = (item, container, popoverTag, sizeRaw, maxSizeRaw, direction) => { + const guardKey = `__fcChatAssistantResize_${direction}`; + if (item[guardKey]) { + return; + } + item[guardKey] = true; + const size = parseFloat(sizeRaw); const maxSize = parseFloat(maxSizeRaw);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize.js` around lines 22 - 31, The fcChatAssistantResize function can run multiple times; add a JS-side idempotence guard at the top of window.fcChatAssistantResize (e.g., check and set a sentinel property like container.__fcChatAssistantResizeInitialized or item.__fcChatAssistantResizeInitialized) so the function returns immediately if the sentinel is present, and set the sentinel before any initialization logic completes; reference the function name window.fcChatAssistantResize and the container/item variables so the guard is easy to locate and will prevent duplicate initialization from this script.src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (2)
221-243: Consider extracting duplicated CSS literal to a constant.The literal
"var(--lumo-font-size-xs)"is repeated 7 times in the badge styling. Extracting it to a constant would improve maintainability.♻️ Suggested refactor
+ private static final String BADGE_SIZE = "var(--lumo-font-size-xs)"; // In setUI(): unreadBadge.getStyle() // ... other styles ... - .setFontSize("var(--lumo-font-size-xs)") + .setFontSize(BADGE_SIZE) // ... - .setMinHeight("var(--lumo-font-size-xs)") - .setMinWidth("var(--lumo-font-size-xs)") - .setHeight("var(--lumo-font-size-xs)") - .setWidth("var(--lumo-font-size-xs)") - .setMaxHeight("var(--lumo-font-size-xs)") - .setMaxWidth("var(--lumo-font-size-xs)") + .setMinHeight(BADGE_SIZE) + .setMinWidth(BADGE_SIZE) + .setHeight(BADGE_SIZE) + .setWidth(BADGE_SIZE) + .setMaxHeight(BADGE_SIZE) + .setMaxWidth(BADGE_SIZE)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 221 - 243, In ChatAssistant, replace the repeated literal "var(--lumo-font-size-xs)" used in the unreadBadge style chain with a single constant: add a private static final String (e.g. LUMO_FONT_SIZE_XS) in the ChatAssistant class and use that constant in all .setMinHeight/.setMinWidth/.setHeight/.setWidth/.setMaxHeight/.setMaxWidth calls (and any other occurrences) instead of the repeated string to improve maintainability and avoid duplication.
106-123: Provide parameterized types for generic collections.Raw generic types should be avoided for type safety.
♻️ Suggested fix
public ChatAssistant(List<T> messages, boolean markdownEnabled) { this.setUI(); - this.content = new VirtualList(); + this.content = new VirtualList<>(); this.messages = messages; this.initializeHeader(); this.initializeFooter(); this.initializeContent(markdownEnabled); this.initializeChatWindow(); } public ChatAssistant() { - this(new ArrayList(), false); + this(new ArrayList<>(), false); } public ChatAssistant(boolean markdownEnabled) { - this(new ArrayList(), markdownEnabled); + this(new ArrayList<>(), markdownEnabled); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 106 - 123, The constructors use raw ArrayList types; replace raw usages with parameterized/delimited generics to ensure type safety—e.g., in the no-arg and markdown-only constructors change new ArrayList() to new ArrayList<>() or new ArrayList<T>() so the ChatAssistant(List<T> messages, ...) calls use a properly typed empty list; update the two lines in the ChatAssistant() and ChatAssistant(boolean) constructors accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 517-520: The private method refreshContent() is unused and
duplicates logic already executed in sendMessage(); remove the entire
refreshContent() method (the private void refreshContent() definition) from
ChatAssistant.java and update any code references if present, ensuring
sendMessage() retains the dataProvider.refreshAll() and scrollToEnd() calls so
behavior is preserved and the class compiles cleanly.
- Around line 361-366: The setFabIcon method currently calls icon.getStyle()
without validating the icon parameter which causes an NPE if null; update
ChatAssistant.setFabIcon to guard against null (e.g., if icon is null call
fab.setIcon(null) and return) otherwise set the width/height using
DEFAULT_FAB_ICON_SIZE and then call fab.setIcon(icon); reference the setFabIcon
method, the icon parameter, fab field, and DEFAULT_FAB_ICON_SIZE when making the
change.
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 221-243: In ChatAssistant, replace the repeated literal
"var(--lumo-font-size-xs)" used in the unreadBadge style chain with a single
constant: add a private static final String (e.g. LUMO_FONT_SIZE_XS) in the
ChatAssistant class and use that constant in all
.setMinHeight/.setMinWidth/.setHeight/.setWidth/.setMaxHeight/.setMaxWidth calls
(and any other occurrences) instead of the repeated string to improve
maintainability and avoid duplication.
- Around line 106-123: The constructors use raw ArrayList types; replace raw
usages with parameterized/delimited generics to ensure type safety—e.g., in the
no-arg and markdown-only constructors change new ArrayList() to new
ArrayList<>() or new ArrayList<T>() so the ChatAssistant(List<T> messages, ...)
calls use a properly typed empty list; update the two lines in the
ChatAssistant() and ChatAssistant(boolean) constructors accordingly.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-resize.js`:
- Around line 34-36: Several shouldDrag functions inside the directionConfig
object (e.g., the 'top' entry's shouldDrag and the other shouldDrag functions
referenced) access overlay.style properties without null-safety; change these to
use optional chaining when reading overlay and overlay.style (e.g.,
overlay?.style?.bottom and overlay?.style?.top) so the checks are safe if
overlay is undefined, and apply the same pattern to all shouldDrag definitions
in directionConfig to avoid runtime errors.
- Around line 22-31: The fcChatAssistantResize function can run multiple times;
add a JS-side idempotence guard at the top of window.fcChatAssistantResize
(e.g., check and set a sentinel property like
container.__fcChatAssistantResizeInitialized or
item.__fcChatAssistantResizeInitialized) so the function returns immediately if
the sentinel is present, and set the sentinel before any initialization logic
completes; reference the function name window.fcChatAssistantResize and the
container/item variables so the guard is easy to locate and will prevent
duplicate initialization from this script.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99121a48-7e56-44b5-b5fb-8d0598da71ff
📒 Files selected for processing (3)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/resources/META-INF/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize.js
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js (1)
130-136:⚠️ Potential issue | 🟠 MajorGuard pointer-capture release in drag teardown.
At line 135,
releasePointerCapturecan throwNotFoundErrorwhen called afterpointercancelevents. The Pointer Events specification automatically releases pointer capture beforepointercancelfires, so a subsequentreleasePointerCapturecall will fail. Apply the guard to safely check capture status:- item.releasePointerCapture(e.pointerId); + if (item.hasPointerCapture?.(e.pointerId)) { + item.releasePointerCapture(e.pointerId); + }The optional chaining ensures compatibility with older browsers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js` around lines 130 - 136, In stopDragging, guard the call to item.releasePointerCapture to avoid NotFoundError when pointercapture was already released (e.g., after pointercancel); locate the stopDragging function and replace the unconditional call to item.releasePointerCapture(e.pointerId) with a safe check using hasPointerCapture (or optional chaining) before calling releasePointerCapture, e.g. only call item.releasePointerCapture(e.pointerId) if item.hasPointerCapture?.(e.pointerId) (or wrap releasePointerCapture in a try/catch as a fallback), keeping the rest of the teardown (isDragging = false, fab.classList.remove('dragging'), item.style.transition, and snapToBoundary()) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pom.xml`:
- Line 19: Remove the unused Maven properties and system property usage for the
Chrome driver: delete the <webdriver.chrome.driver> property and the related
${drivers.dir} property reference from the pom (remove the
webdriver.chrome.driver and drivers.dir entries) and also remove any code that
passes -Dwebdriver.chrome.driver=${drivers.dir}/chromedriver to the JVM (e.g.,
in surefire/failsafe or plugin configuration). Target the property named
webdriver.chrome.driver and any references to drivers.dir and the
-Dwebdriver.chrome.driver system property so tests rely solely on
WebDriverManager.chromedriver().setup() and new ChromeDriver().
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 372-377: The setFabIcon(Component, int) method currently allows
non-positive sizes which produce invalid CSS; add a lower-bound check in
setFabIcon to reject or clamp those values (e.g., if size <= 0 throw new
IllegalArgumentException("size must be > 0") or compute int safeSize =
Math.max(1, size) and then use Math.min(safeSize, DEFAULT_FAB_SIZE)); update the
lines that compute width/height to use this validated/clamped size and keep
setting fab.setIcon(icon) unchanged.
- Around line 566-582: Add the `@Deprecated` annotation to the two deprecated
methods so IDEs and the compiler flag their usage: annotate both
setMinimized(boolean) and isMinimized() with `@Deprecated` immediately above their
declarations (matching the existing javadoc `@deprecated` tags) and ensure any
related javadoc `@since/`@see notes remain intact; no new imports are required
since Deprecated is in java.lang.
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 89-97: Clamp computed xMax and yMax to not fall below margin
before using them to adjust position: when calculating xMax/yMax (based on
screenWidth/screenHeight and itemRect), ensure you take max(margin, xMax) and
max(margin, yMax) so that for very small viewports the fallback does not produce
negative bounds; then use those clamped bounds when setting
position.x/position.y and call updatePosition(); reference variables xMax, yMax,
margin, position, itemRect, screenWidth, screenHeight, and updatePosition to
locate and update the logic.
---
Duplicate comments:
In `@src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js`:
- Around line 130-136: In stopDragging, guard the call to
item.releasePointerCapture to avoid NotFoundError when pointercapture was
already released (e.g., after pointercancel); locate the stopDragging function
and replace the unconditional call to item.releasePointerCapture(e.pointerId)
with a safe check using hasPointerCapture (or optional chaining) before calling
releasePointerCapture, e.g. only call item.releasePointerCapture(e.pointerId) if
item.hasPointerCapture?.(e.pointerId) (or wrap releasePointerCapture in a
try/catch as a fallback), keeping the rest of the teardown (isDragging = false,
fab.classList.remove('dragging'), item.style.transition, and snapToBoundary())
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7fc47554-adf5-4da7-bdaf-fa38f782046a
⛔ 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 (8)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/resources/META-INF/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/frontend/fc-chat-assistant-resize.jssrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.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.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.java
- src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java
javier-godoy
requested changes
Apr 13, 2026
github-project-automation
Bot
moved this from To Do
to In Progress
in Flowing Code Addons
paodb
requested changes
Apr 13, 2026
paodb
requested changes
Apr 13, 2026
Member
There was a problem hiding this comment.
js files should be located in /src/main/resources/META-INF/resources/frontend as Vaadin documentation explains https://vaadin.com/docs/latest/flow/advanced/loading-resources#flow.resource-cheat-sheet (table 3). The same for css files.
Totremont
force-pushed
the
drop-react
branch
2 times, most recently
from
4c028b4 to
d1c6804
Compare
Contributor
Author
Commit now contains "!" to signal a breaking change and a description: d3f7fef |
Contributor
Author
|
scardanzan
requested changes
Apr 15, 2026
Member
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css (1)
45-54: Consider showing resize cursors before drag starts.If
.activeis only added during resizing, users won’t see resize cursors on hover. Consider applying the cursor to the handle classes by default and reserving.activefor drag-state styling.Possible CSS refinement
-.fc-chat-assistant-resize-bottom-left.active { cursor: sw-resize; } -.fc-chat-assistant-resize-bottom-right.active { cursor: se-resize; } -.fc-chat-assistant-resize-top-left.active { cursor: nw-resize; } -.fc-chat-assistant-resize-top-right.active { cursor: ne-resize; } +.fc-chat-assistant-resize-bottom-left { cursor: sw-resize; } +.fc-chat-assistant-resize-bottom-right { cursor: se-resize; } +.fc-chat-assistant-resize-top-left { cursor: nw-resize; } +.fc-chat-assistant-resize-top-right { cursor: ne-resize; } /* Specific cursors for the sides */ -.fc-chat-assistant-resize-bottom.active { cursor: s-resize; } -.fc-chat-assistant-resize-top.active { cursor: n-resize; } -.fc-chat-assistant-resize-left.active { cursor: w-resize; } -.fc-chat-assistant-resize-right.active { cursor: e-resize; } +.fc-chat-assistant-resize-bottom { cursor: s-resize; } +.fc-chat-assistant-resize-top { cursor: n-resize; } +.fc-chat-assistant-resize-left { cursor: w-resize; } +.fc-chat-assistant-resize-right { cursor: e-resize; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css` around lines 45 - 54, The CSS only sets resize cursors for selectors with the .active class, so users won't see resize cursors on hover; update the rules to apply cursor styles to the base handle classes (e.g. .fc-chat-assistant-resize-bottom-left, .fc-chat-assistant-resize-bottom-right, .fc-chat-assistant-resize-top-left, .fc-chat-assistant-resize-top-right and the side handles .fc-chat-assistant-resize-bottom, -top, -left, -right) instead of or in addition to the .active variants, and keep .active reserved for drag-state styling (e.g. visual feedback) so the cursor appears before dragging starts.src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (3)
697-699: RedundantMath.maxingetUnreadMessages.
setUnreadMessagesalready clampsunreadMessagesto[0, 99], and the field is initialized to0, sounreadMessagescan never be negative. TheMath.max(unreadMessages, 0)is dead defensive logic.public int getUnreadMessages() { - return Math.max(unreadMessages, 0); + return unreadMessages; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 697 - 699, The getUnreadMessages method contains redundant defensive logic; since the field unreadMessages is initialized to 0 and setUnreadMessages already clamps values to [0,99], remove the unnecessary Math.max call in ChatAssistant.getUnreadMessages and return the unreadMessages field directly (keep setUnreadMessages and the unreadMessages field unchanged to preserve the clamping behavior).
581-598: ConsiderforRemoval = trueon the deprecated minimized API.The javadoc states these are replaced by
setOpened(boolean)/isOpened(), andsetAvatarProvider(line 686) already usesforRemoval = true. AligningsetMinimized/isMinimizedwould surface stronger deprecation warnings in IDEs/compilers to match the stated intent.- `@Deprecated`(since = "5.0.0") + `@Deprecated`(since = "5.0.0", forRemoval = true) public void setMinimized(boolean minimized) { ... - `@Deprecated`(since = "5.0.0") + `@Deprecated`(since = "5.0.0", forRemoval = true) public boolean isMinimized() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 581 - 598, The `@Deprecated` annotations on setMinimized and isMinimized should indicate intended removal: update their annotations to use `@Deprecated`(since = "5.0.0", forRemoval = true) so they match the stated replacement API (setOpened/isOpened) and the existing setAvatarProvider deprecation style; change the annotations on the methods setMinimized and isMinimized accordingly and update any related javadoc if needed to remain consistent.
360-371: Simpler and more obviously-correct flag logic.The two sequential
if (!this['flag'])blocks are functionally equivalent to a single guarded block: the flag is simply set after%2$sexecutes (the executable doesn't touchthis[flag]). Fewer branches make the intent clearer and avoid the appearance of a race.♻️ Proposed simplification
protected void addComponentRefreshedListener(String uniqueFlag, String executable, Serializable... parameters) { this.getElement().executeJs( String.format( """ - if(!this['%1$s']) { %2$s } - if(!this['%1$s']) { - this['%1$s'] = '%1$s'; - }; + if (!this['%1$s']) { + this['%1$s'] = true; + %2$s + } """, uniqueFlag, executable), parameters ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 360 - 371, The current addComponentRefreshedListener builds JS with two sequential if-checks for the same flag; simplify by emitting a single guarded block so the executable runs only if the flag is not set and then set the flag immediately after running it. Update the String.format passed to getElement().executeJs in addComponentRefreshedListener to one if(!this['%1$s']) { %2$s; this['%1$s'] = '%1$s'; } and keep the same parameters (uniqueFlag, executable, parameters) so behavior is unchanged but intent is clearer and race-like appearance removed.src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js (2)
41-238: Heavy duplication across the 8 direction configs.The eight direction entries repeat near-identical logic:
shouldDragis a boolean combination of four "edge rules" (top/bottom/left/right), andhandleResize/setupDrag/cleanupDragare compositions of four primitives (top/bottom/left/right). Consider deriving each direction from an axis descriptor to cut ~200 lines and make it easier to keep the corner variants in sync.♻️ Sketch of the refactor
const edgeRules = { top: () => overlay?.style?.bottom && !overlay?.style?.top, bottom: () => !overlay?.style?.bottom && overlay?.style?.top, left: () => overlay?.style?.right && !overlay?.style?.left, right: () => overlay?.style?.left && !overlay?.style?.right, }; const axisOps = { top: { resize: (e) => { const rect = container.getBoundingClientRect(); const newHeight = (rect.top - e.clientY) + container.clientHeight; if (newHeight >= minHeight && newHeight <= maxHeight) container.style.height = newHeight + 'px'; }, setup: () => { item.style.height = maxSize + 'px'; item.style.marginTop = -(maxSize / 2) + 'px'; }, cleanup: () => { item.style.height = size + 'px'; item.style.marginTop = ''; }, }, // bottom / left / right ... }; const directions = { 'top': ['top'], 'bottom': ['bottom'], 'left': ['left'], 'right': ['right'], 'top-right': ['top', 'right'], 'bottom-right': ['bottom', 'right'], 'bottom-left': ['bottom', 'left'], 'top-left': ['top', 'left'], }; const axes = directions[direction]; const config = { shouldDrag: () => axes.every(a => edgeRules[a]()), handleResize: (e) => axes.forEach(a => axisOps[a].resize(e)), setupDrag: () => axes.forEach(a => axisOps[a].setup()), cleanupDrag: () => axes.forEach(a => axisOps[a].cleanup()), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js` around lines 41 - 238, directionConfig duplicates the same edge-based logic across eight entries; replace it with a composable approach: define edgeRules (top/bottom/left/right) that reference overlay.style, define axisOps for each edge that implement resize/setup/cleanup using container/item and min/max sizes, then map directions (e.g. 'top-right' -> ['top','right']) and build each directionConfig entry by composing shouldDrag (axes.every(edgeRules)), handleResize (axes.forEach(axisOps.resize)), setupDrag (axes.forEach(axisOps.setup)) and cleanupDrag (axes.forEach(axisOps.cleanup)). Use the existing symbols directionConfig, shouldDrag, handleResize, setupDrag, cleanupDrag, container, item, minHeight/minWidth/maxHeight/maxWidth, edgeRules, axisOps and directions to locate and replace the repeated blocks.
246-257: Overlay lookup relies on a polling fallback and a magic 2 s timeout.If the overlay is not mounted by the next animation frame, the code just waits 2 seconds and tries once more. On a slow page or when the popover opens later than 2 s after
fcChatAssistantResizeis invoked,overlaystaysundefined,shouldDrag()permanently returns a falsy value, and the resize handle silently stops working. AMutationObserver(or waiting onchatWindow.opened) would be more deterministic than a fixed delay.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js` around lines 246 - 257, The overlay lookup in fetchOverlay currently relies on requestAnimationFrame plus a single setTimeout(2000) fallback which can miss later mounts; replace the fixed 2s retry with a MutationObserver that watches the document (childList: true, subtree: true) for additions matching the popoverTag/overlayTag (or observe the popover element for changes) and call fetchOverlay when matching nodes appear, then disconnect the observer once overlay is found; alternatively, if the chat component exposes an opened event/property (e.g. chatWindow.opened), listen for that and invoke fetchOverlay instead of using setTimeout; update fetchOverlay (and the initialization flow where requestAnimationFrame is called) to attach this observer/listener and ensure it cleans up after resolving overlay so shouldDrag() will reliably work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 687-691: The setAvatarProvider method currently calls
avatarProvider.get() and passes its result to setFabIcon which will throw a
NullPointerException; update setAvatarProvider to handle a null avatar from the
user-supplied SerializableSupplier<Avatar> by either treating null as a no-op
(i.e., leave the current icon) or by throwing a clearer IllegalArgumentException
with an explicit message; specifically, inside setAvatarProvider (and before
calling setFabIcon), check the result of avatarProvider.get() and if null either
return without calling setFabIcon or throw a descriptive exception (e.g.,
"Avatar provider returned null") so callers see a clear error instead of the
downstream Objects.requireNonNull from setFabIcon.
- Around line 138-201: The chatWindow.addOpenedChangeListener is being added on
every onAttach causing listener leaks; capture the returned Registration (e.g.,
a new field like openedChangeRegistration) when calling
chatWindow.addOpenedChangeListener inside onAttach or—better—move the
chatWindow.addOpenedChangeListener call into the constructor so it runs once;
then remove it in onDetach by calling openedChangeRegistration.remove() (or
null-check then remove) to unregister the listener; reference
addOpenedChangeListener, onAttach, onDetach, chatWindow,
addComponentRefreshedListener and use a Registration field to manage lifecycle.
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 40-76: The resize listener added via window.addEventListener(...)
captures item, fab, and container and is never removed, leaking the detached
ChatAssistant subtree; fix by storing the handler (or an AbortController signal)
when attaching and remove the listener when the component is torn down (e.g., in
your component's disconnectedCallback or teardown routine) — reference the
resize handler that calls snapToBoundary(), and ensure you call
window.removeEventListener("resize", handler) (or abort the controller) so the
closure no longer retains item, fab, container and allows GC.
- Around line 130-141: In stopDragging, guard the call to
item.releasePointerCapture(e.pointerId) with a hasPointerCapture check to avoid
throwing InvalidPointerId when the pointer was already released (e.g.,
pointercancel/pointerleave); locate the stopDragging function and replace the
unconditional releasePointerCapture call with a conditional like if
(item.hasPointerCapture(e.pointerId)) { item.releasePointerCapture(e.pointerId);
} so the rest of the logic (isDragging=false, class removal on fab, transitions,
snapToBoundary(), and isClickOnlyEvent() -> root.$server?.onClick()) executes
unchanged.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js`:
- Around line 290-297: The stopDragging handler calls config.cleanupDrag()
unconditionally causing hover-only pointerleave to trigger a cleanup; modify
stopDragging (the function named stopDragging) to first capture the current
isDragging value into a local variable, then only call config.cleanupDrag() and
releasePointerCapture when that captured value is true (i.e., a real drag was in
progress), and finally reset isDragging to false and remove the 'active' class;
ensure you still call item.releasePointerCapture(e.pointerId) only when the drag
was active to avoid unnecessary style writes.
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Around line 697-699: The getUnreadMessages method contains redundant defensive
logic; since the field unreadMessages is initialized to 0 and setUnreadMessages
already clamps values to [0,99], remove the unnecessary Math.max call in
ChatAssistant.getUnreadMessages and return the unreadMessages field directly
(keep setUnreadMessages and the unreadMessages field unchanged to preserve the
clamping behavior).
- Around line 581-598: The `@Deprecated` annotations on setMinimized and
isMinimized should indicate intended removal: update their annotations to use
`@Deprecated`(since = "5.0.0", forRemoval = true) so they match the stated
replacement API (setOpened/isOpened) and the existing setAvatarProvider
deprecation style; change the annotations on the methods setMinimized and
isMinimized accordingly and update any related javadoc if needed to remain
consistent.
- Around line 360-371: The current addComponentRefreshedListener builds JS with
two sequential if-checks for the same flag; simplify by emitting a single
guarded block so the executable runs only if the flag is not set and then set
the flag immediately after running it. Update the String.format passed to
getElement().executeJs in addComponentRefreshedListener to one if(!this['%1$s'])
{ %2$s; this['%1$s'] = '%1$s'; } and keep the same parameters (uniqueFlag,
executable, parameters) so behavior is unchanged but intent is clearer and
race-like appearance removed.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js`:
- Around line 41-238: directionConfig duplicates the same edge-based logic
across eight entries; replace it with a composable approach: define edgeRules
(top/bottom/left/right) that reference overlay.style, define axisOps for each
edge that implement resize/setup/cleanup using container/item and min/max sizes,
then map directions (e.g. 'top-right' -> ['top','right']) and build each
directionConfig entry by composing shouldDrag (axes.every(edgeRules)),
handleResize (axes.forEach(axisOps.resize)), setupDrag
(axes.forEach(axisOps.setup)) and cleanupDrag (axes.forEach(axisOps.cleanup)).
Use the existing symbols directionConfig, shouldDrag, handleResize, setupDrag,
cleanupDrag, container, item, minHeight/minWidth/maxHeight/maxWidth, edgeRules,
axisOps and directions to locate and replace the repeated blocks.
- Around line 246-257: The overlay lookup in fetchOverlay currently relies on
requestAnimationFrame plus a single setTimeout(2000) fallback which can miss
later mounts; replace the fixed 2s retry with a MutationObserver that watches
the document (childList: true, subtree: true) for additions matching the
popoverTag/overlayTag (or observe the popover element for changes) and call
fetchOverlay when matching nodes appear, then disconnect the observer once
overlay is found; alternatively, if the chat component exposes an opened
event/property (e.g. chatWindow.opened), listen for that and invoke fetchOverlay
instead of using setTimeout; update fetchOverlay (and the initialization flow
where requestAnimationFrame is called) to attach this observer/listener and
ensure it cleans up after resolving overlay so shouldDrag() will reliably work.
In
`@src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css`:
- Around line 45-54: The CSS only sets resize cursors for selectors with the
.active class, so users won't see resize cursors on hover; update the rules to
apply cursor styles to the base handle classes (e.g.
.fc-chat-assistant-resize-bottom-left, .fc-chat-assistant-resize-bottom-right,
.fc-chat-assistant-resize-top-left, .fc-chat-assistant-resize-top-right and the
side handles .fc-chat-assistant-resize-bottom, -top, -left, -right) instead of
or in addition to the .active variants, and keep .active reserved for drag-state
styling (e.g. visual feedback) so the cursor appears before dragging starts.
🪄 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: 877b6a11-e448-403e-b883-99c00b5af5da
⛔ 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 (14)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.javasrc/main/resources/META-INF/frontend/react/animated-fab.tsxsrc/main/resources/META-INF/frontend/styles/chat-assistant-styles.csssrc/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/fcChatAssistantConnector.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/chatassistant/ChatAssistantDemo.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.java
💤 Files with no reviewable changes (3)
- src/main/resources/META-INF/frontend/react/animated-fab.tsx
- src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css
- src/main/resources/META-INF/resources/frontend/fcChatAssistantConnector.js
Rewrite ChatAssistant from a React-based ReactAdapterComponent to a pure Vaadin Java component extending Div. Replace the React animated FAB with a native Vaadin Button and Popover, and add custom JavaScript for drag and resize behavior. BREAKING CHANGE: ChatAssistant no longer extends ReactAdapterComponent and no longer implements ClickNotifier. The component no longer depends on React-related npm packages (react-draggable, @mui/material, @mui/icons-material, @emotion/react, @emotion/styled).
|
github-project-automation
Bot
moved this from In Progress
to Pending release
in Flowing Code Addons
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #57
Summary by CodeRabbit
New Features
setFabIcon()Refactor
Deprecation
setMinimized()andisMinimized()methods deprecated; useopen()/close()/isOpened()insteadsetAvatarProvider()deprecated; usesetFabIcon()instead