Drop React FAB dependency by replacing ReactAdapter with vanilla component #58
Open
Drop React FAB dependency by replacing ReactAdapter with vanilla component #58
Conversation
WalkthroughMajor architectural rewrite of ChatAssistant component from React-based ReactAdapterComponent to vanilla Vaadin Div-based implementation. Removes React dependency, introduces draggable/resizable window with FAB control, updates public API, and bumps version to 5.0.0-SNAPSHOT with new JavaScript utilities for client-side interactions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
🚥 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)
📝 Coding Plan for PR comments
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
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Outdated
Show resolved
Hide resolved
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Outdated
Show resolved
Hide resolved
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-left.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css
Outdated
Show resolved
Hide resolved
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.java
Outdated
Show resolved
Hide resolved
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
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-left.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top-left.js
Outdated
Show resolved
Hide resolved
src/main/resources/META-INF/frontend/styles/fc-chat-assistant-style.css
Outdated
Show resolved
Hide resolved
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
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Show resolved
Hide resolved
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Show resolved
Hide resolved
Comment on lines
+101
to
+129
| item.addEventListener('pointerdown', (e) => { | ||
| isDragging = true; | ||
| fab.classList.add('dragging'); | ||
| item.setPointerCapture(e.pointerId); | ||
| item.style.transition = sizeTransition; | ||
| initialPosition.x = position.x; | ||
| initialPosition.y = position.y; | ||
| }); | ||
|
|
||
| item.addEventListener('pointermove', (e) => { | ||
| if (!isDragging) return; | ||
| const itemRect = fab.getBoundingClientRect(); | ||
| // Calculate position from right and bottom edges | ||
| position.x = screenWidth - e.clientX - (itemRect.width / 2); | ||
| position.y = screenHeight - e.clientY - (itemRect.height / 2); | ||
|
|
||
| updatePosition(); | ||
| }); | ||
|
|
||
| item.addEventListener('pointerup', (e) => { | ||
| isDragging = false; | ||
| item.style.transition = snapTransition + ', ' + sizeTransition; | ||
| fab.classList.remove('dragging'); | ||
| item.releasePointerCapture(e.pointerId); | ||
| snapToBoundary(); | ||
| if (isClickOnlyEvent()) { | ||
| root.$server?.onClick(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/main/resources/META-INF/frontend/fc-chat-assistant-movement.js | head -150Repository: FlowingCode/ChatAssistant
Length of output: 5045
Add pointercancel event handler to reset drag state.
Line 121 is the only code path that resets isDragging, removes the dragging class, and releases pointer capture. If the pointer gesture is canceled by the browser or OS, the element remains in a stuck dragging state—the isDragging flag stays true, the dragging class persists on fab, and pointer capture is never released, blocking further pointer interactions.
Proposed fix
+ function stopDragging(e, fireClick) {
+ isDragging = false;
+ item.style.transition = snapTransition + ', ' + sizeTransition;
+ fab.classList.remove('dragging');
+ if (item.hasPointerCapture?.(e.pointerId)) {
+ item.releasePointerCapture(e.pointerId);
+ }
+ snapToBoundary();
+ if (fireClick && isClickOnlyEvent()) {
+ root.$server?.onClick();
+ }
+ }
+
- item.addEventListener('pointerup', (e) => {
- isDragging = false;
- item.style.transition = snapTransition + ', ' + sizeTransition;
- fab.classList.remove('dragging');
- item.releasePointerCapture(e.pointerId);
- snapToBoundary();
- if (isClickOnlyEvent()) {
- root.$server?.onClick();
- }
- });
+ item.addEventListener('pointerup', (e) => stopDragging(e, true));
+ item.addEventListener('pointercancel', (e) => stopDragging(e, false));🤖 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 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.
src/main/resources/META-INF/frontend/fc-chat-assistant-resize-bottom-right.js
Outdated
Show resolved
Hide resolved
Comment on lines
+48
to
+93
| item.addEventListener('pointerenter', (e) => { | ||
| if (shouldDrag()) { | ||
| item.classList.add('active'); | ||
| item.setPointerCapture(e.pointerId); | ||
| minHeight = container.style.minHeight ? parseFloat(container.style.minHeight) : 0; | ||
| maxHeight = container.style.maxHeight ? parseFloat(container.style.maxHeight) : Infinity; | ||
| } | ||
| else { | ||
| item.classList.remove('active'); | ||
| } | ||
| }); | ||
|
|
||
| item.addEventListener('pointerdown', (_) => { | ||
| isDragging = shouldDrag(); | ||
| if (isDragging) { | ||
| item.style.height = maxSize + 'px'; | ||
| item.style.marginTop = -(maxSize / 2) + 'px'; | ||
| } | ||
| }); | ||
|
|
||
| item.addEventListener('pointermove', (e) => { | ||
| if (!isDragging) return; | ||
| const offsetY = container.getBoundingClientRect().top - e.clientY; | ||
| const newHeight = offsetY + container.clientHeight; | ||
| if (newHeight >= minHeight && newHeight <= maxHeight) { | ||
| container.style.height = newHeight + 'px'; | ||
| } | ||
| }); | ||
|
|
||
| item.addEventListener('pointerup', (e) => { | ||
| isDragging = false; | ||
| item.style.height = size + 'px'; | ||
| item.style.marginTop = ''; | ||
| if (item.hasPointerCapture(e.pointerId)) { | ||
| item.releasePointerCapture(e.pointerId); | ||
| } | ||
| }); | ||
|
|
||
| item.addEventListener('pointerleave', (e) => { | ||
| isDragging = false; | ||
| item.style.height = size + 'px'; | ||
| item.style.marginTop = ''; | ||
| if (item.hasPointerCapture(e.pointerId)) { | ||
| item.releasePointerCapture(e.pointerId); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/resources/META-INF/frontend/fc-chat-assistant-resize-top.js | sed -n '40,100p'Repository: FlowingCode/ChatAssistant
Length of output: 2290
Handle canceled resize gestures and remove sticky hover state.
This interaction only resets on pointerup/pointerleave. If the browser sends pointercancel (e.g., system interrupt, gesture conflict), the handle stays expanded and isDragging remains stale. Additionally, the active class is never removed after a drag completes—it persists until you re-enter without drag capability. Pull the reset logic into a shared stopDragging() handler, wire it to pointercancel, and clear the active class there.
Proposed fix
+ function stopDragging(e) {
+ isDragging = false;
+ item.classList.remove('active');
+ item.style.height = size + 'px';
+ item.style.marginTop = '';
+ if (item.hasPointerCapture?.(e.pointerId)) {
+ item.releasePointerCapture(e.pointerId);
+ }
+ }
+
item.addEventListener('pointerup', (e) => {
- isDragging = false;
- item.style.height = size + 'px';
- item.style.marginTop = '';
- if (item.hasPointerCapture(e.pointerId)) {
- item.releasePointerCapture(e.pointerId);
- }
+ stopDragging(e);
});
- item.addEventListener('pointerleave', (e) => {
- isDragging = false;
- item.style.height = size + 'px';
- item.style.marginTop = '';
- if (item.hasPointerCapture(e.pointerId)) {
- item.releasePointerCapture(e.pointerId);
- }
- });
+ item.addEventListener('pointercancel', stopDragging);
+ item.addEventListener('pointerleave', stopDragging);🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 52-52: Prefer Number.parseFloat over parseFloat.
🤖 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.js` around
lines 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.
|
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
Comment on lines
+361
to
+366
| public void setFabIcon(Component icon) { | ||
| icon.getStyle() | ||
| .setWidth(DEFAULT_FAB_ICON_SIZE + "px") | ||
| .setHeight(DEFAULT_FAB_ICON_SIZE + "px"); | ||
| fab.setIcon(icon); | ||
| } |
There was a problem hiding this comment.
Add null check for icon parameter in setFabIcon.
The method accesses icon.getStyle() without validation, which will throw NullPointerException if called with null.
🛡️ Suggested fix
public void setFabIcon(Component icon) {
+ Objects.requireNonNull(icon, "Icon cannot be null");
icon.getStyle()
.setWidth(DEFAULT_FAB_ICON_SIZE + "px")
.setHeight(DEFAULT_FAB_ICON_SIZE + "px");
fab.setIcon(icon);
}🤖 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 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.
Comment on lines
517
to
520
| private void refreshContent() { | ||
| content.getDataProvider().refreshAll(); | ||
| content.scrollToEnd(); | ||
| this.content.getDataProvider().refreshAll(); | ||
| this.content.scrollToEnd(); | ||
| } |
There was a problem hiding this comment.
Remove unused private method refreshContent.
This method is never called in the class. The functionality it provides is duplicated inline in sendMessage() (lines 529-531).
🧹 Suggested removal
- private void refreshContent() {
- this.content.getDataProvider().refreshAll();
- this.content.scrollToEnd();
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void refreshContent() { | |
| content.getDataProvider().refreshAll(); | |
| content.scrollToEnd(); | |
| this.content.getDataProvider().refreshAll(); | |
| this.content.scrollToEnd(); | |
| } |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 517-517: Remove this unused private "refreshContent" method.
🤖 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 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.
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
2 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
Deprecations
Chores