Improvements: add mobile mode and other features - #72
Conversation
Swap the markdown-editor-addon MarkdownViewer for Vaadin's built-in Markdown component in ChatMessage.
Add the directional resize handles, window sizing with clamped min/max bounds applied to the popover content part, size persistence across close/reopen, the screen-size threshold tracking, and the resize direction indicators.
Replace the stale markdown-editor rule with rules that collapse the rendered markdown's outer block margins and tighten inter-block gaps.
Close #64 Add a Lombok @builder constructor and extend the component with: - FAB icon (default inline chatbot icon, custom overloads), sizing, and theme variants (color pass-through; LUMO_SMALL/LARGE drive the diameter). - FAB placement: setFabPosition/resetFabPosition, margin, setFabMovable, and setFabAnchoredToViewport for bounded placement. - Window control: setWindowResizable, setResizeIndicatorsVisible, initial size, and min/max bounds honored on open and while resizing; the window shrinks to its min height and clamps to the overlay. - Display mode: setMode/setMobileMode with a full-screen mobile dialog, breakpoint auto-switching (opt-in), and addModeChangedListener. - addScreenSizeListener for chat-window size threshold crossings.
Commented basic/lazy-loading/markdown/generative demos using the current API.
A FAB configuration demo (custom icon, size and color variants, section titles, movable/resizable/indicator toggles with notifications) and an in-a-box demo with a container-anchored FAB positioned via setFabPosition.
Desktop/mobile modes with breakpoint auto-switching, manual setMode, a mode-changed listener with notification, FAB position reset, and a chat window screen-size threshold listener.
Expand the feature list and replace the outdated getting-started example with current builder/setter-based tutorials covering messaging, FAB styling, window sizing, and responsive mobile mode.
WIP: address FAB API and behavior review feedback Introduce a FabVariant enum for FAB theming. Make isFabMovable() report the effective current state. Enforce a single ChatAssistant per UI. Use --lumo-warning-contrast-color as the badge text. Extract the --fc-min/max-* CSS property names into constants. Drop the duplicate setOpenOnClick(false). Add @SInCE 5.1.0 tags to the new public API. WIP: fix frontend listener cleanup and stale overlay references WIP: add unit tests for ChatAssistant pure-Java logic Cover parseFabMargin fallbacks, setUnreadMessages clamping to 0-99, addScreenSizeListener argument validation, and the FabVariant to ButtonVariant mapping, in the lightweight style of SerializationTest. WIP: add window sizing options to the builder Expose width, height, maxWidth and maxHeight on the ChatAssistant builder so the initial window size and its max bounds can be configured at construction time. WIP: apply Google Java Style WIP: normalize copyright year range WIP: support both Lumo and Aura themes Make the FAB color variants cross-theme: SUCCESS and ERROR now also carry an Aura accent CSS class (aura-accent-green/red) alongside the Lumo theme variant, since Aura styles accent colors via class rather than the theme attribute; PRIMARY stays on the cross-theme primary token and LUMO_CONTRAST is documented as Lumo-only. Replace the hardcoded --lumo-* tokens on the unread badge and in fc-chat-assistant-style.css (shadows, border radius, contrast) with cross-theme fallback chains (var(--lumo-x, var(--aura-y, <literal>))), mirroring the pattern already used in fc-chat-message-styles.css. WIP: report effective FAB drag state from isFabMovable isFabMovable() now returns fabMovable && fabAnchoredToViewport, matching the client drag gate (movable && anchored). A container-anchored FAB cannot be dragged, so the getter no longer reports true in that configuration; it already returned false in mobile mode. Adds a unit test covering the anchored/movable combinations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fix background-thread hygiene in demos ChatAssistantDemo replaces the leaked java.util.Timer (non-daemon, never cancelled) with a daemon-threaded ScheduledExecutorService that is shut down on detach. ChatAssistantGenerativeDemo runs its streaming task on a dedicated daemon executor instead of the shared ForkJoinPool.commonPool(), so its blocking sleeps no longer tie up common-pool threads; the executor is shut down on detach. Streaming behaviour and UI.access updates are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: simplify public javadoc, drop client-internal details Rewrite the javadoc on isFabMovable, setResizeIndicatorsVisible and the FabVariant enum so it describes behaviour from the caller's point of view rather than leaking client-side implementation details (the movable/anchored attributes, the Aura CSS-class-vs-theme-attribute mechanism, and Lumo design tokens). No behavioural change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: encapsulate FabVariant theming internals Make the ButtonVariant mapping and Aura CSS class private to FabVariant and expose intent-level applyColorTo(Button)/removeColorFrom(Button) instead of the toButtonVariant()/getAuraClass() accessors, so the underlying ButtonVariant and theme-class details no longer leak into the public API. ChatAssistant delegates color application to the variant; size handling is unchanged. Tests now verify the applied theme variant and Aura class through a real Button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: clarify only default functionality comply WIP: fix invalid CSS rejected by lightningcss (Vaadin 25.2) Check that the css follows the rules of lightningcss. Close #69 Close #67 WIP: make introduced variables private instead of protected WIP: store FAB icon once as a single Component field Merge the redundant fabIcon/fabIconComponent fields into a single Component field, applying instanceof checks only where an SvgIcon is specifically required. Addresses PR #68 review r3545904693. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reset unread badge text color on whitespace-only input setUnreadBadgeColors guarded the background with isBlank() but the text color with isEmpty(), so a whitespace-only color was applied verbatim instead of falling back to the default. Use isBlank() for both. Addresses PR #68 review r3546279320. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: clear recycled message row content while loading updateMessage gated all content rewriting behind !isLoading(), so when the ComponentRenderer recycled a pooled VirtualList row to show a loading placeholder the previous message's text/markdown stayed visible next to the spinner. Always rewrite the content, blanking it while loading. Addresses PR #68 review r3546279308. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make screen-size listener work in mobile mode addScreenSizeListener always observed the desktop popover overlay Div, but in MOBILE mode the chat content is moved into the mobile Dialog and the overlay is empty (0x0), so the listener never fired. Observe the container surface instead (it moves between both windows and a ResizeObserver follows it across DOM moves), and re-deliver the size on mobile dialog open for parity with the popover. Addresses PR #68 review r3546279298. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: load default FAB icon lazily and degrade gracefully The default chatbot SVG was read in a static-final initializer, so a missing/unreadable resource threw at class-load time (ExceptionInInitializer Error, then NoClassDefFoundError on every later reference) even for callers supplying their own icon. Load it lazily on first use, cache the result, and fall back to no icon with a warning instead of throwing. Addresses PR #68 review r3546279303. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reject non-pixel lengths in window-size setters The client clamps window sizes numerically (parseFloat(value) + "px"), so a relative unit like "50%" or "30vw" passed to the String size setters was silently reinterpreted as pixels and clamped to the min bound. Validate the String width/height/min/max setters up front and fail with a clear message, matching the documented absolute-units contract. Addresses PR #68 review r3546279289. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: keep duplicate ChatAssistant inert instead of throwing on attach Attaching a second ChatAssistant to a UI threw IllegalStateException, which could crash attach-before-detach route navigation or a dialog-over-view. Log a warning and leave the extra instance hidden and unwired instead, so it never competes with the already-attached instance and navigation is not broken. Addresses PR #68 review r3546279283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: re-init FAB movement after reattach movement.js disconnectedCallback removed the resize listener but never cleared the __fcChatAssistantMovement / fc-chat-assistant-drag-listener init guards, so after a detach/reattach addComponentRefreshedListener skipped re-initialization and the resize handler was never re-added (FAB could end up stranded off-screen). Clear both guards on disconnect. Addresses PR #68 review r3546279330. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: re-init resize handles after reattach resize.js disconnectedCallback tore down the window resize listener, style observer and overlay-lookup timeout but never cleared the per-direction init guard on the durable resizer Div (nor the server refresh guard), so on reattach fcChatAssistantResize early-returned and the resize handlers/ indicators were never re-established. Clear both guards on disconnect (which only fires on a genuine detach, not on popover reopen). Addresses PR #68 review r3546279335. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make builder constructor private and use primitive flags The all-args builder constructor was public with many boxed Boolean flags that had to be null-defaulted in setUI. Make it private (construction is via ChatAssistant.builder() or the public legacy constructors) and switch the flags to primitive boolean so callers cannot pass null. A small partial ChatAssistantBuilder seeds the real defaults for the flags whose default is not false, and setUI no longer null-coalesces. Addresses PR #68 review r3545748934. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make internal resizer styling helpers private applyGenericResizerStyle and setResizerClass are internal helpers used only within ChatAssistant to build/toggle the resize handles; they need not be part of the protected extension surface. Make them private. Addresses PR #68 review r3546022009. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: inline size-variant check in FAB theme variant methods add/removeFabThemeVariants branched on variant.isSizeVariant() and then assumed the variant was LARGE or SMALL (variant == LARGE ? ... : SMALL). Inline the explicit SMALL/LARGE check so that assumption is provable at the call site and a future non-size variant cannot fall into the size branch. isSizeVariant() remains as public API. Addresses PR #68 review r3545985941. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: use diamond operator in SerializationTest Drop the redundant explicit type argument on the right-hand side; the target type already provides it, per the style guide's diamond preference. Addresses PR #68 review r3546149078. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: merge duplicated popover/dialog content-part CSS rules The popover ::part(content) declaration block was duplicated across the Vaadin 24 (vaadin-popover-overlay) and Vaadin 25 (vaadin-popover) selectors, and likewise for the dialog pair. Combine each pair into a single comma-separated rule (valid under lightningcss) so future tweaks live in one place. Addresses PR #68 review r3546279367. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: share overlay resolution between resize.js helpers fetchOverlay and fcChatAssistantContentPart each hardcoded the same overlay selector chain (popover class -> shadowRoot -> vaadin-popover-overlay, with a getElementsByClassName fallback). Extract a single resolveOverlay(popoverTag) helper so the DOM structure is described once. Addresses PR #68 review r3546279356. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: deduplicate setFabIcon overloads Both setFabIcon(Component) and setFabIcon(Component, int) repeated the icon assignment, fab.setIcon and applyIconSize sequence. Extract a private applyFabIcon(icon, size) helper. The no-size overload keeps its own path (getFabIconSize() can be 0, which is valid here but rejected by the sized overload's size > 0 guard). Addresses PR #68 review r3546279361. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: defer window size/constraint push until the popover is open Each of the six window size/bound setters ran an executeJs that polled the popover content part up to 20x100ms; at construction the popover is closed, so a fully-configured builder spun ~6 pointless ~2s polling loops. Store the desired size in the --fc-* custom properties on the durable overlay Div from Java and only push to the client when the popover is open (the open listener already re-applies everything via fcChatAssistantRestoreWindowSize). The now-unused fcChatAssistantSetWindowSize JS helper is removed. Addresses PR #68 review r3546279350. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: hold resize handles in a direction-keyed map The eight resizer Divs were separate fields enumerated across three sites (edge styling, resizable class toggling, and client registration). Replace them with a single Map<String,Div> keyed by direction plus a RESIZER_DIRECTIONS list, and iterate that one collection at each site, so adding or renaming a handle is a single-line change. Per-direction edge styling moves into applyResizerEdgeStyle. Addresses PR #68 review r3546279344. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: run frontend teardown via an animated-fab custom element The root <animated-fab> element has no backing web component, so the root.disconnectedCallback assigned by the movement and resize modules was never invoked by the browser: the window resize listeners, style observer, overlay-lookup timeout and screen-size/mobile observers leaked on detach and the init guards stayed set (blocking reattach). Register a minimal animated-fab custom element whose disconnectedCallback runs teardown callbacks accumulated on root.__fcCleanups, and have both modules push their cleanup there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: delegate setFabIcon(Component) to the sized overload setFabIcon(Component) now calls setFabIcon(icon, getFabIconSize()) instead of duplicating the assignment/install/size logic. getFabIconSize() returns at least 1px so it never trips the sized overload's size > 0 guard when fabSize is small. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: don't nudge the FAB on a sub-threshold click pointermove mutated position before the sensitivity check returned, so a click with tiny movement was committed by stopDragging -> snapToBoundary -> updatePosition, shifting the FAB. Compute a candidate position and only commit it (and update the DOM) once the move exceeds the drag sensitivity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: reject negative FAB margin parseFabMargin accepted a negative value (e.g. "-5"), which would pull the FAB off the edge. Fall back to the default for negatives, and cover it with a test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: abort generative demo streaming on interrupt streamWords swallowed InterruptedException (re-set the flag but kept emitting), so after onDetach's executor.shutdownNow() the loop kept calling ui.access on a detached view. Replace the stream with a plain loop whose Thread.sleep propagates the interrupt and aborts streaming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fix FAB theme-variant example in README addFabThemeVariants takes FabVariant, not ButtonVariant. Use FabVariant.LARGE/LUMO_CONTRAST in the snippet and correct the feature-list wording so the sample compiles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: correct @SInCE tags on window sizing/unread API Remove @SInCE 5.1.0 from the String window-size setters, getUnreadMessages and setUnreadMessages(int) (all present since 5.0.0), and add @SInCE 5.1.0 to setWindowResizable/isWindowResizable, which are new in 5.1.0 (also normalizing the malformed isWindowResizable javadoc). Verified against tag chat-assistant-addon-5.0.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: move FAB color application out of FabVariant FabVariant.applyColorTo/removeColorFrom were public methods that mutated a Button, leaking the apply logic. Make FabVariant a data-only enum exposing getButtonVariant()/getAuraClass(), and apply/remove the color from private ChatAssistant methods. Tests now assert the variant's data mapping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: fill empty @PARAM in ChatMessage.updateMessage javadoc Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: trim redundant javadoc and reflow mangled comments Remove the Lombok-internals explanation on ChatAssistantBuilder, the breaking-change rationale on the mobile breakpoint, and the redundant single-instance comment in onAttach (the code and warning already say it). Reflow comments that had been wrapped into orphaned one-word lines (these render verbatim in the @demosource viewer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: drop write-only minWidth/minHeight fields The minWidth/minHeight fields were only ever assigned; the real state is the --fc-min-width/--fc-min-height custom properties on the overlay Div (read by fcChatAssistantApplyConstraints). Remove the dead fields and set the custom properties directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: target the animated-fab tag in integration tests The component root tag is animated-fab, but the ViewIT selector, the ChatAssistantElement page object and the demo CSS still referenced the old chat-bot tag. Update them so the ITs resolve the component (and, with the custom-element registration, ViewIT's upgrade assertion now holds). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: cover screen-size listener state in SerializationTest Register an addScreenSizeListener before serializing so the screen-size listener map (ScreenSizeListenerEntry) is exercised by the round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: simplify px concatenation and add a color literal fallback Drop the redundant String.valueOf(...) in the int window-size setters (int + "px" already yields a String), and add a literal fallback to the who-is-typing color var chain so it degrades when neither the Lumo nor Vaadin token is defined. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: assert animated-fab upgrade instead of shadow-root content Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: accept any CSS length for window size, drop px-only validation Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: make internal FAB helpers private Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: place @SInCE before @param/@return per FC javadoc convention Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> WIP: document FAB movable no-op and defensive message copy Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the FAB being trapped by an ancestor that establishes a containing block for fixed descendants (e.g. Aura AppLayout navbar backdrop-filter): the anchored wrapper is lifted to document.body so position:fixed resolves against the viewport, and restored to its slot on detach. Close #70
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesChatAssistant feature expansion
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
23-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
com.vaadin:vaadin-markdown-flowwith version${vaadin.version}.com.vaadin:vaadin-coredoes not providecom.vaadin.flow.component.markdown.Markdowntransitively.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pom.xml` at line 23, Add the com.vaadin:vaadin-markdown-flow dependency to the Maven dependency configuration, using the existing ${vaadin.version} property for its version so Markdown is available explicitly alongside vaadin-core.Source: MCP tools
🧹 Nitpick comments (12)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java (1)
53-75: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd regression coverage for Markdown with lazy loading.
No existing integration test combines Markdown rendering with a lazy
DataProvider. Add an integration test that loads long Markdown messages across pages and scrolls through them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java` around lines 53 - 75, Add integration coverage around ChatAssistantLazyLoadingDemo that enables Markdown rendering, supplies long Markdown messages through the lazy DataProvider across multiple pages, and scrolls through the loaded messages. Assert that Markdown content remains correctly rendered while paging, preserving the existing lazy-loading behavior.src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (7)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSkip the default icon load when the builder supplies an icon.
The field initializer at Line 93 creates the default icon for every instance.
setUIthen replaces it at Line 366 when afabIconis supplied. The data URI is cached statically, so the cost is small, but the discardedSvgIconinstance is avoidable. InitializefabIconinsetUIonly.Also applies to: 366-366
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` at line 93, Remove the eager createDefaultFabIcon() initializer from the fabIcon field, and initialize fabIcon within setUI only when the builder has not supplied an icon; preserve the supplied icon unchanged.
984-991: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the
fcChatAssistantResetPositioncall like the other client calls.
setFabAnchoredToViewportuseswindow.fcChatAssistantPortalFab && ...andaddScreenSizeListeneruseswindow.fcChatAssistantScreenSizeOff?.(...).resetFabPositioncalls the global directly.setFabPositionandsetModecan run before the module executes in some lifecycles, which then logs a client-sideTypeError. Add the same optional-call guard for consistency.🛡️ Proposed change
this.getElement() .executeJs( - "window.fcChatAssistantResetPosition($0, $1, $2);", + "window.fcChatAssistantResetPosition?.($0, $1, $2);", fabWrapper.getElement(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 984 - 991, Update resetFabPosition to guard the window.fcChatAssistantResetPosition invocation with the same optional-call pattern used by the other client calls, so it safely no-ops when the module has not executed yet.
1084-1128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the dimension keys used by
applyWindowSize.SonarCloud reports the literals
"height"and"width"as duplicated three times each.applyWindowSizealso compares the dimension by string, which is easy to break. Two small private helpers (or an enum) remove the string comparison.♻️ Proposed change
- private void applyWindowSize(String dimension, String value) { + private void applyWindowSize(String cssProperty, String value) { if (value == null) { return; } - overlay.getStyle().set("height".equals(dimension) ? CSS_HEIGHT : CSS_WIDTH, value); + overlay.getStyle().set(cssProperty, value); applyWindowConstraints(); }Callers then pass
CSS_HEIGHTorCSS_WIDTHdirectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 1084 - 1128, Refactor applyWindowSize and its callers setWindowHeight/setWindowWidth to use the existing CSS_HEIGHT and CSS_WIDTH constants directly instead of the string literals "height" and "width". Remove the dimension string comparison in applyWindowSize, using the passed CSS property to apply the value while preserving null handling and constraint application.Source: Linters/SAST tools
265-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWiden the builder
fabIconparameter toComponent.
setFabIcon(Component)accepts any component, and thefabIconfield is typedComponent. The builder restricts the icon toSvgIcon, so avaadin-iconor anImagecannot be supplied at construction time. Widening the parameter now avoids a breaking signature change later.♻️ Proposed change
`@Builder` private ChatAssistant( - SvgIcon fabIcon, + Component fabIcon, boolean resizable,
setUIneeds the same parameter type change at Line 317.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` at line 265, Update the builder’s fabIcon parameter from SvgIcon to Component, matching the fabIcon field and setFabIcon(Component) API so any component can be provided during construction. Also change the setUI parameter at the referenced builder method to Component, preserving the existing assignment and builder behavior.
264-280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a parameter object for the construction configuration.
Both the builder constructor and
setUItake 16 and 14 positional parameters. SonarCloud flagssetUIfor the parameter count. Adjacent parameters of the same type (String minWidth, String minHeight, String width, String height, String maxWidth, String maxHeight) are easy to transpose in future edits, and the legacy constructor at Line 192 already passes eight barenullvalues. A small private configuration record passed tosetUIwould remove the positional coupling without changing the public API.Also applies to: 316-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 264 - 280, Introduce a private configuration record/class for the construction and UI settings, including the related sizing and positioning values, and pass that object through the private ChatAssistant constructor and setUI instead of their long positional parameter lists. Update the builder construction path and the legacy constructor’s null/default setup to create this configuration object, preserving the existing public API and behavior while eliminating adjacent same-typed argument transposition risk.Source: Linters/SAST tools
1438-1440: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
getUnreadMessagesclamping is redundant.
setUnreadMessagesalready stores a value in the 0–99 range, and the field starts at0.Math.max(unreadMessages, 0)can never change the result. Return the field directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 1438 - 1440, Update getUnreadMessages to return unreadMessages directly instead of applying Math.max, relying on setUnreadMessages and the field’s initialization to maintain the valid nonnegative range.
1627-1648: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSet the per-key refresh guard that the client clears, or drop the guard from the client.
fcChatAssistantScreenSizeOffandfcChatAssistantScreenSizeOffAllinsrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.jsclearroot['fc-chat-assistant-screen-size-' + key]. No Java code sets that flag;applyScreenSizeListenercallsexecuteJsdirectly instead ofaddComponentRefreshedListener. The client cleanup is therefore a no-op today. Remove the guard handling in the client, or route the registration throughaddComponentRefreshedListenerwith that flag name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java` around lines 1627 - 1648, Fix the screen-size listener cleanup contract between addScreenSizeListener and the client-side fcChatAssistantScreenSizeOff/fcChatAssistantScreenSizeOffAll handlers. Either remove their clearing of the per-key fc-chat-assistant-screen-size-{key} guard, or update applyScreenSizeListener to register through addComponentRefreshedListener using that exact guard name so Java sets it before cleanup.src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js (1)
418-425: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSimplify the length-matching regular expression.
SonarCloud flags
^\s*\d*\.?\d+(px)?\s*$for super-linear backtracking.\d*\.?\d+allows several ways to match the same digits. An unambiguous alternation removes the backtracking and keeps the accepted set identical.♻️ Proposed refactor
+// Matches a plain number or px length: "12", "12px", "1.5px", ".5px". +const FC_PX_LENGTH = /^\s*(?:\d+(?:\.\d+)?|\.\d+)(?:px)?\s*$/;Use
FC_PX_LENGTH.test(rawValue)at Line 420 andFC_PX_LENGTH.test(valueRaw)at Line 437.Also applies to: 436-447
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js` around lines 418 - 425, Replace the ambiguous inline length regex checks in the num helper and the corresponding value parsing block with the shared FC_PX_LENGTH pattern. Preserve the existing fallback and numeric parsing behavior while using FC_PX_LENGTH.test for both rawValue and valueRaw validation sites.Source: Linters/SAST tools
src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js (2)
247-257: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDisconnect the
IntersectionObserverduring teardown.The observer is disconnected only after the FAB reports a non-zero width. If the host detaches while the FAB is still hidden, the observer stays connected. Register it with the teardown callbacks so the detach path releases it.
♻️ Proposed refactor
} else { const observer = new IntersectionObserver((_, obs) => { if (fcChatAssistantSize(fab).width > 0) { obs.disconnect(); applyCorner(); } }); observer.observe(fab); + root.__fcCleanups.push(() => observer.disconnect()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js` around lines 247 - 257, Update the IntersectionObserver created in the hidden-FAB branch of the movement initialization flow to register its disconnect operation with the existing teardown callbacks. Ensure teardown disconnects the observer even when the FAB never reports a non-zero width, while preserving the current disconnect-and-apply behavior once it becomes visible.
192-207: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe sensitivity gate stays active for the whole drag.
The comparison is always against
initialPosition, not against the previous committed position. After the pointer leaves the threshold zone the FAB tracks the cursor, but if the user drags back toward the start the position freezes inside a 25px zone around the origin (DEFAULT_DRAG_SENSITIVITY). A one-shot flag limits the gate to the start of the gesture.♻️ Proposed refactor
item.addEventListener('pointerdown', (e) => { isDragging = fab.hasAttribute('movable') && fab.hasAttribute('anchored'); if (!isDragging) return; + hasMoved = false;- if (Math.abs(nextX - initialPosition.x) < sensitivity + if (!hasMoved + && Math.abs(nextX - initialPosition.x) < sensitivity && Math.abs(nextY - initialPosition.y) < sensitivity) { return; } + hasMoved = true;Declare
let hasMoved = false;next toisDragging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js` around lines 192 - 207, Update the pointermove drag logic alongside isDragging to declare and use a hasMoved flag. Apply the sensitivity check against initialPosition only until the first movement exceeds the threshold, then set hasMoved and continue updating position on subsequent moves, including movements back toward the origin; reset the flag when each drag gesture ends.src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabVariant.java (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
isSizeVariant()at the call sites.
addFabThemeVariantsandremoveFabThemeVariantsinsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java(Lines 758-787) testvariant == FabVariant.SMALL || variant == FabVariant.LARGEinstead of calling this accessor. The size classification is then encoded in two places. If a further size variant is added, the enum flag alone will not change behavior.Note also that
getButtonVariant()returnsLUMO_SMALL/LUMO_LARGEfor the size variants, butChatAssistantnever applies those button variants because size variants take the resize branch. Consider documenting that, or passingnullfor the size variants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabVariant.java` around lines 59 - 66, Update addFabThemeVariants and removeFabThemeVariants in ChatAssistant to use FabVariant.isSizeVariant() instead of explicitly comparing against SMALL and LARGE, keeping size classification centralized in the enum. Also align getButtonVariant’s contract with the size-variant resize branch by either documenting that its LUMO size result is not applied there or returning null for size variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java`:
- Around line 84-98: Update ChatMessage.setMessage to clear the user-name,
user-img, and time attributes when message.getName(), message.getAvatar(), or
message.getMessageTime() is null. Handle avatar independently of the name so it
is always updated or cleared, and preserve formatting for non-null message
times.
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 331-348: Use the same box measurement for both initial and
observer deliveries in the notify flow around entry.observer and overlayDiv:
observe and measure overlayDiv consistently, or consistently use its content
box, so padding and border are treated identically. Replace the mismatched
getBoundingClientRect() initial measurement with the measurement corresponding
to the ResizeObserver callback while preserving the existing zero-size and
threshold logic.
- Around line 39-54: Update fcChatAssistantPortalFab so fabWrapper remains under
its Flow parent instead of being appended to document.body; adjust the anchoring
behavior to avoid disrupting Flow’s sibling insertion and preserve correct fixed
positioning, or otherwise make Flow’s addChildren/insertion path portal-aware so
body-level siblings are never passed to animated-fab.insertBefore.
In
`@src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css`:
- Around line 45-48: Scope the `vaadin-popover-overlay::part(overlay)`
border-radius selector to the `fc-chat-assistant-popover` class, matching the
containment pattern used by the other rules in this stylesheet. Keep the
existing fallback radius values unchanged.
---
Outside diff comments:
In `@pom.xml`:
- Line 23: Add the com.vaadin:vaadin-markdown-flow dependency to the Maven
dependency configuration, using the existing ${vaadin.version} property for its
version so Markdown is available explicitly alongside vaadin-core.
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`:
- Line 93: Remove the eager createDefaultFabIcon() initializer from the fabIcon
field, and initialize fabIcon within setUI only when the builder has not
supplied an icon; preserve the supplied icon unchanged.
- Around line 984-991: Update resetFabPosition to guard the
window.fcChatAssistantResetPosition invocation with the same optional-call
pattern used by the other client calls, so it safely no-ops when the module has
not executed yet.
- Around line 1084-1128: Refactor applyWindowSize and its callers
setWindowHeight/setWindowWidth to use the existing CSS_HEIGHT and CSS_WIDTH
constants directly instead of the string literals "height" and "width". Remove
the dimension string comparison in applyWindowSize, using the passed CSS
property to apply the value while preserving null handling and constraint
application.
- Line 265: Update the builder’s fabIcon parameter from SvgIcon to Component,
matching the fabIcon field and setFabIcon(Component) API so any component can be
provided during construction. Also change the setUI parameter at the referenced
builder method to Component, preserving the existing assignment and builder
behavior.
- Around line 264-280: Introduce a private configuration record/class for the
construction and UI settings, including the related sizing and positioning
values, and pass that object through the private ChatAssistant constructor and
setUI instead of their long positional parameter lists. Update the builder
construction path and the legacy constructor’s null/default setup to create this
configuration object, preserving the existing public API and behavior while
eliminating adjacent same-typed argument transposition risk.
- Around line 1438-1440: Update getUnreadMessages to return unreadMessages
directly instead of applying Math.max, relying on setUnreadMessages and the
field’s initialization to maintain the valid nonnegative range.
- Around line 1627-1648: Fix the screen-size listener cleanup contract between
addScreenSizeListener and the client-side
fcChatAssistantScreenSizeOff/fcChatAssistantScreenSizeOffAll handlers. Either
remove their clearing of the per-key fc-chat-assistant-screen-size-{key} guard,
or update applyScreenSizeListener to register through
addComponentRefreshedListener using that exact guard name so Java sets it before
cleanup.
In
`@src/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabVariant.java`:
- Around line 59-66: Update addFabThemeVariants and removeFabThemeVariants in
ChatAssistant to use FabVariant.isSizeVariant() instead of explicitly comparing
against SMALL and LARGE, keeping size classification centralized in the enum.
Also align getButtonVariant’s contract with the size-variant resize branch by
either documenting that its LUMO size result is not applied there or returning
null for size variants.
In
`@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`:
- Around line 247-257: Update the IntersectionObserver created in the hidden-FAB
branch of the movement initialization flow to register its disconnect operation
with the existing teardown callbacks. Ensure teardown disconnects the observer
even when the FAB never reports a non-zero width, while preserving the current
disconnect-and-apply behavior once it becomes visible.
- Around line 192-207: Update the pointermove drag logic alongside isDragging to
declare and use a hasMoved flag. Apply the sensitivity check against
initialPosition only until the first movement exceeds the threshold, then set
hasMoved and continue updating position on subsequent moves, including movements
back toward the origin; reset the flag when each drag gesture ends.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.js`:
- Around line 418-425: Replace the ambiguous inline length regex checks in the
num helper and the corresponding value parsing block with the shared
FC_PX_LENGTH pattern. Preserve the existing fallback and numeric parsing
behavior while using FC_PX_LENGTH.test for both rawValue and valueRaw validation
sites.
In
`@src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.java`:
- Around line 53-75: Add integration coverage around
ChatAssistantLazyLoadingDemo that enables Markdown rendering, supplies long
Markdown messages through the lazy DataProvider across multiple pages, and
scrolls through the loaded messages. Assert that Markdown content remains
correctly rendered while paging, preserving the existing lazy-loading behavior.
🪄 Autofix
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 Plus
Run ID: 18891fa4-b6b7-44c5-9a65-94b04f5b8a92
⛔ Files ignored due to path filters (2)
src/main/resources/META-INF/resources/icons/chatbot.svgis excluded by!**/*.svgsrc/test/resources/META-INF/resources/chatbot.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.gitignoreREADME.mdpom.xmlsrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/ChatAssistantMode.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabPosition.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/FabVariant.javasrc/main/java/com/flowingcode/vaadin/addons/chatassistant/model/Message.javasrc/main/resources/META-INF/VAADIN/package.propertiessrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.jssrc/main/resources/META-INF/resources/frontend/fc-chat-assistant-resize.jssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.csssrc/main/resources/META-INF/resources/frontend/styles/fc-chat-message-styles.csssrc/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.javasrc/test/java/com/flowingcode/vaadin/addons/DemoLayout.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantBoxDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantFabConfigDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLazyLoadingDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantLogicTest.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantMarkdownDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantModeDemo.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomChatMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/CustomMessage.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/DemoView.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/AbstractViewTest.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/BasicIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/ViewIT.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatAssistantElement.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/it/po/ChatBubbleElement.javasrc/test/java/com/flowingcode/vaadin/addons/chatassistant/test/SerializationTest.javasrc/test/resources/META-INF/frontend/styles/chat-assistant-styles-demo.css
💤 Files with no reviewable changes (1)
- src/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.java
| public void setMessage(T message) { | ||
| this.message = message; | ||
| updateMessage(message); | ||
| if (message.getName()!=null) { | ||
| if (message.getName() != null) { | ||
| this.setUserName(message.getName()); | ||
| if (message.getAvatar()!=null) { | ||
| if (message.getAvatar() != null) { | ||
| this.setUserImg(message.getAvatar()); | ||
| } | ||
| } | ||
| if (message.getMessageTime()!=null) { | ||
| String formattedTime = message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm")); | ||
| if (message.getMessageTime() != null) { | ||
| String formattedTime = | ||
| message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm")); | ||
| this.setTime(formattedTime); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear metadata when a virtual-list row is reused.
When setMessage receives a message with a null name, avatar, or message time, it skips the setter but does not remove the previous user-name, user-img, or time attribute. A recycled row can display metadata from another message. Clear each attribute in its null branch, and handle the avatar independently of the name.
Proposed fix
- if (message.getName() != null) {
+ if (message.getName() != null) {
this.setUserName(message.getName());
- if (message.getAvatar() != null) {
- this.setUserImg(message.getAvatar());
- }
+ } else {
+ getElement().removeAttribute("user-name");
+ }
+ if (message.getAvatar() != null) {
+ this.setUserImg(message.getAvatar());
+ } else {
+ getElement().removeAttribute("user-img");
}
if (message.getMessageTime() != null) {
String formattedTime =
message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm"));
this.setTime(formattedTime);
+ } else {
+ getElement().removeAttribute("time");
}📝 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.
| public void setMessage(T message) { | |
| this.message = message; | |
| updateMessage(message); | |
| if (message.getName()!=null) { | |
| if (message.getName() != null) { | |
| this.setUserName(message.getName()); | |
| if (message.getAvatar()!=null) { | |
| if (message.getAvatar() != null) { | |
| this.setUserImg(message.getAvatar()); | |
| } | |
| } | |
| if (message.getMessageTime()!=null) { | |
| String formattedTime = message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm")); | |
| if (message.getMessageTime() != null) { | |
| String formattedTime = | |
| message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm")); | |
| this.setTime(formattedTime); | |
| } | |
| } | |
| public void setMessage(T message) { | |
| this.message = message; | |
| updateMessage(message); | |
| if (message.getName() != null) { | |
| this.setUserName(message.getName()); | |
| } else { | |
| getElement().removeAttribute("user-name"); | |
| } | |
| if (message.getAvatar() != null) { | |
| this.setUserImg(message.getAvatar()); | |
| } else { | |
| getElement().removeAttribute("user-img"); | |
| } | |
| if (message.getMessageTime() != null) { | |
| String formattedTime = | |
| message.getMessageTime().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm")); | |
| this.setTime(formattedTime); | |
| } else { | |
| getElement().removeAttribute("time"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java`
around lines 84 - 98, Update ChatMessage.setMessage to clear the user-name,
user-img, and time attributes when message.getName(), message.getAvatar(), or
message.getMessageTime() is null. Handle avatar independently of the name so it
is always updated or cleared, and preserve formatting for non-null message
times.
| // Lifts the FAB wrapper to <body> while it is anchored to the viewport, so its position:fixed | ||
| // resolves against the viewport rather than an ancestor containing block. Any ancestor with | ||
| // transform/filter/backdrop-filter/perspective/contain/will-change (e.g. Aura's AppLayout navbar) | ||
| // would otherwise trap the wrapper. When it is not anchored, the wrapper is returned to its home | ||
| // slot so position:absolute stays relative to its container. Idempotent. | ||
| window.fcChatAssistantPortalFab = (item, anchored) => { | ||
| if (anchored) { | ||
| if (item.parentNode !== document.body) { | ||
| // Remember where the wrapper lived so it can be put back on teardown / un-anchor. | ||
| item.__fcHome = item.__fcHome || { parent: item.parentNode, next: item.nextSibling }; | ||
| document.body.appendChild(item); | ||
| } | ||
| } else if (item.parentNode === document.body) { | ||
| fcChatAssistantRestoreFab(item); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Vaadin Flow move server-side element to document.body client-side DOM error
💡 Result:
The error "Cannot move a node from one state tree to another" in Vaadin Flow occurs when you attempt to attach a server-side component (or element) that is already attached to one UI (or state tree) to a different location or a different UI instance [1][2]. Vaadin's state tree architecture requires that a component belong to exactly one parent/tree at a time to maintain synchronization between the server and the client [1]. To move or re-attach an element to a new parent, including the document body, follow these guidelines: 1. Remove from Current Parent First: If the element is already part of the component hierarchy, you must explicitly detach it from its current parent before moving it. Use the removeFromParent() method on the element or removeFromTree() if it is being moved across state trees (such as during refresh cycles) [3][1]. 2. Accessing the Body Element: To append a component to the document.body specifically, you can access the UI's element. Note that UI.getCurrent().getElement() typically represents the tag in a standard Vaadin application [4][5][6]. Use the following pattern within an onAttach lifecycle method to ensure the component is available: @Override protected void onAttach(AttachEvent attachEvent) { super.onAttach(attachEvent); // Ensure the element is removed from its current parent if necessary myElement.removeFromParent; // Append to the UI's root element (the body) this.getUI.get.getElement.appendChild(myElement); } 3. Preventing the Error: - Avoid manual DOM manipulation that bypasses Vaadin's internal state management. If you move an element using client-side JavaScript, the server-side state tree will not automatically reflect this change, leading to synchronization errors [7]. - If you are encountering this error during page refreshes or with @PreserveOnRefresh, ensure that you are not re-adding the same component instance without proper cleanup [1]. - If you need to modify the body element itself (e.g., adding global CSS classes or styles), prefer using AppShellConfigurator or IndexHtmlRequestListener rather than manual manipulation of document.body via getElement() during runtime, as this is more resilient and follows best practices for bootstrap modification [5][8]. Top results: [5][1][8][2][9]
Citations:
- 1: https://stackoverflow.com/questions/75019718/vaadin-flow-14-cant-move-a-node-from-one-state-tree-to-another-error-when-refre
- 2: https://vaadin.com/forum/t/cant-move-a-node-from-one-state-tree-to-another-error-in-vaadin-10/158093
- 3: Allow dom manipulation with template elements vaadin/flow#3391
- 4: https://vaadin.com/forum/t/element-api-against-document-document-documentelement-html/178367
- 5: https://vaadin.com/forum/t/correct-way-to-access-body-element/158008
- 6: UI.getCurrent().getComponent() in WebComponent returns body-Element vaadin/flow#19133
- 7: Element positioning via style doesn't work after position has been edited via JS previously vaadin/flow#19702
- 8: https://vaadin.com/docs/latest/flow/advanced/modifying-the-bootstrap-page
- 9: https://vaadin.com/forum/t/absolute-positioning/157847
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(ChatAssistant|.*chat.*assistant.*|.*movement.*|.*fab.*)' | head -200
printf '%s\n' '--- relevant symbols and annotations ---'
rg -n --glob '*.java' --glob '*.js' --glob '*.html' \
'fcChatAssistantPortalFab|fabWrapper|PreserveOnRefresh|animated-fab|ChatAssistant|appendChild|document\.body|restoreFab' .Repository: FlowingCode/ChatAssistant
Length of output: 30547
🏁 Script executed:
#!/bin/bash
set -e
files=$(git ls-files | rg 'ChatAssistant|chat-assistant|fc-chat-assistant|movement' || true)
for f in $files; do
printf '\n--- %s (%s lines) ---\n' "$f" "$(wc -l < "$f")"
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline "$f" 2>/dev/null | head -120 || true
fi
done
printf '%s\n' '--- Java hierarchy and lifecycle references ---'
rg -n -C 4 --glob '*.java' \
'class .*ChatAssistant|extends .*Component|`@PreserveOnRefresh`|add\(|remove\(|removeFromParent|ElementFactory|animated-fab|fabWrapper' $files 2>/dev/null || trueRepository: FlowingCode/ChatAssistant
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- ChatAssistant construction and lifecycle ---'
sed -n '350,525p' src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
printf '%s\n' '--- FAB anchoring ---'
sed -n '910,960p' src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
printf '%s\n' '--- movement portal and teardown ---'
sed -n '20,78p' src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js
sed -n '100,165p' src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js
printf '%s\n' '--- project Flow/Vaadin versions ---'
rg -n -C 3 'vaadin|flow.version|com.vaadin' pom.xml gradle* .mvn 2>/dev/null | head -160
printf '%s\n' '--- PreserveOnRefresh usage ---'
rg -n --glob '*.java' --glob '*.js' 'PreserveOnRefresh|preserveOnRefresh|refresh' src/mainRepository: FlowingCode/ChatAssistant
Length of output: 20849
🌐 Web query:
Vaadin Flow client-side StateTree server-side element moved manually DOM parent add remove child mapping
💡 Result:
In Vaadin Flow, the StateTree acts as the single source of truth for the client-side DOM, and it is strictly synchronized by the server [1][2]. You cannot manually manipulate the client-side DOM directly and expect the server-side StateTree to reflect those changes automatically; doing so will lead to state inconsistency [2]. The framework manages the synchronization between the server-side Java components (represented by the Element API) and the client-side browser DOM [3][2]. When you perform operations on the server side using methods like Element.appendChild, Element.removeFromParent, or Element.insertChild, the framework generates and tracks these state changes in the StateTree [4][5][6][7]. Before the next response is sent to the client, the framework collects these modifications and transmits them as JSON instructions to the client-side rendering engine, which then updates the actual DOM [2][8]. If you move an element manually (e.g., via JavaScript in the browser), the server-side StateTree remains unaware of the structural change. To ensure proper synchronization, you must perform such operations through the Java Element API [3][5]: 1. To move an element: Remove the element from its current parent using Element.removeFromParent and add it to the new parent using Element.appendChild (or similar) [5]. 2. For custom DOM interactions: If you require advanced client-side manipulation that the Element API does not cover, you should use the ExecuteJavaScript or Element.executeJs methods to perform the DOM manipulation and, if necessary, communicate the outcome back to the server so that the server-side state can be updated to match [2]. Attempting to bypass this flow by directly modifying the DOM without informing the server will result in the client-side and server-side states diverging, which often breaks event handling, state tracking, and future updates [2].
Citations:
- 1: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/StateTree.html
- 2: https://vaadin.com/docs/latest/flow/what-is-flow
- 3: https://vaadin.com/docs/latest/flow/component-internals/basic
- 4: https://github.com/vaadin/flow/blob/455661e3bb94f88411ad45c06623fac0eec48839/flow-server/src/main/java/com/vaadin/flow/dom/Element.java
- 5: https://vaadin.com/docs/latest/flow/component-internals/container
- 6: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/24.5.7/com/vaadin/flow/dom/Element.html
- 7: https://origin.vaadin.com/api/platform/current/com/vaadin/flow/dom/Node.html
- 8: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/24.5.13/com/vaadin/flow/internal/StateTree.html
🌐 Web query:
site:github.com/vaadin/flow "Cannot move a node" client DOM StateTree
💡 Result:
The error "Can't move a node from one state tree to another" is a common java.lang.IllegalStateException in Vaadin Flow applications [1][2][3]. It occurs when the framework detects an attempt to attach a component or element to a new UI or state tree while it is still associated with a different one [1][4]. This is typically caused by unintended component reuse, often stemming from the following scenarios: 1. Improper Bean Scoping: Components are frequently reused if they are defined as static members or as singleton-scoped beans in Spring [1][5]. Because a Vaadin component instance can only be attached to one location at a time, referencing the same instance across multiple views or UIs triggers this error [1][6]. 2. Component Re-use in Renderers: Using a single component instance inside a renderer (like a grid renderer) instead of generating a new instance for each row/item [6]. 3. Lifecycle Issues with @PreserveOnRefresh: When using the @PreserveOnRefresh annotation, components may be improperly managed during page reloads, especially if logic tries to re-add components that haven't been correctly detached [2][7]. How to resolve and debug: - Check for Singleton/Static usage: Ensure that views and components are not defined as static fields or singleton beans. Use appropriate scopes (e.g., @UIScope or prototype scope) where necessary to ensure each UI instance gets its own component instances [5]. - Verify Component Creation: Ensure that any component used within a renderer or repeated structure is created via a factory function or a new instance constructor rather than being a pre-created shared object [6]. - Use removeFromTree: If you are intentionally moving a component from one parent to another, ensure you call component.getElement().removeFromTree() before re-adding it to the new parent [1][7][8]. Note that this method is intended for specific use cases and should be used with caution [8][4]. - Debugging: If the cause is unclear, inspect the stack trace to identify the "offending component" [1]. In complex applications, check if browser tab duplication or page refreshes are inadvertently triggering old logic on a new UI instance [5].
Citations:
- 1: Add clarifying information to the error message about moving nodes to another state tree vaadin/flow#9376
- 2: Can't move a node from one state tree to another when using @PreserveOnRefresh vaadin/flow#14609
- 3: Vaadin application throws exception upon screen refresh vaadin/flow#15436
- 4: Allow moving components from one UI to another UI vaadin/flow#5165
- 5: Missing resource will cause unintuitive exception with Router vaadin/flow#4336
- 6: StateNode#handleOnAttach exception message should be improved vaadin/flow#7813
- 7: Error "Can't move a node from one state tree to another." when re-adding a previously removed component after refreshing with @PreserveOnRefresh vaadin/flow#8286
- 8: Cannot move element to a different UI: can't change id once it has been assigned vaadin/flow#18390
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import json, urllib.parse, urllib.request
queries = [
'repo:vaadin/flow "getDomNode()" "StateNode"',
'repo:vaadin/flow "NodeFeature" "appendChild"',
'repo:vaadin/flow "Cannot move a node from one state tree to another"',
]
for q in queries:
url = 'https://api.github.com/search/code?q=' + urllib.parse.quote(q)
print('\n---', q, '---')
try:
with urllib.request.urlopen(url, timeout=20) as r:
data = json.load(r)
for item in data.get('items', [])[:10]:
print(item.get('html_url'))
except Exception as e:
print(type(e).__name__, e)
PY
printf '%s\n' '--- repository metadata ---'
curl -L --fail --silent --show-error \
https://api.github.com/repos/vaadin/flow/contents/flow-client/src/main/java/com/vaadin/client/flow \
| jq -r '.[].path' | head -100Repository: FlowingCode/ChatAssistant
Length of output: 857
🌐 Web query:
Vaadin Flow StateNode NodeFeature client DOM insert child sibling source
💡 Result:
In Vaadin Flow, the StateNode architecture acts as the synchronization layer between the server-side component tree and the client-side DOM [1][2]. When an element insertion occurs (e.g., via Element.insertChild), the framework utilizes specific NodeFeatures to manage these structural changes [3][4]. The primary mechanism for inserting children or siblings involves: 1. NodeFeatures: A StateNode organizes data into distinct NodeFeatures [1][5]. For DOM structure, features like NodeList are commonly used to maintain ordered collections of child StateNodes [6]. Each feature is responsible for recording its own changes [5][6]. 2. Change Collection: When a modification occurs, the change is recorded on the StateNode. During the server-to-client synchronization phase, UidlWriter triggers the StateTree to collect these changes from all dirty features [7]. 3. Client-Side Processing: Changes are serialized as JSON and sent to the client [7]. The client-side TreeChangeProcessor receives these updates [8]. Specifically, splice changes (CHANGE_TYPE_SPLICE) are processed to insert, move, or remove nodes within the client-side DOM tree [8]. 4. Siblings: Siblings are managed implicitly through the order maintained within the parent's child NodeList feature [6]. When a node is inserted at a specific index, the framework calculates the necessary splice operations to maintain consistent DOM positioning between the server and the client [3][4][8]. In summary, server-side DOM operations are delegated to StateNodes, which use NodeFeatures to track state changes. These changes are subsequently synchronized to the client via a JSON-based protocol, where the client-side infrastructure interprets splice commands to perform the corresponding DOM manipulation [8][7].
Citations:
- 1: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/StateNode.html
- 2: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/StateTree.html
- 3: https://vaadin.com/api/platform/current/com/vaadin/flow/dom/ElementStateProvider.html
- 4: https://origin.vaadin.com/api/platform/current/com/vaadin/flow/dom/Node.html
- 5: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/24.5.7/com/vaadin/flow/internal/nodefeature/NodeFeature.html
- 6: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/package-summary.html
- 7: https://github.com/vaadin/flow/blob/master/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlWriter.java
- 8: https://github.com/vaadin/flow/blob/master/flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java
🏁 Script executed:
#!/bin/bash
set -e
base=https://raw.githubusercontent.com/vaadin/flow/master
for path in \
flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java \
flow-client/src/main/java/com/vaadin/client/flow/StateNode.java \
flow-client/src/main/java/com/vaadin/client/flow/NodeList.java \
flow-client/src/main/java/com/vaadin/client/flow/NodeFeature.java
do
printf '\n--- %s ---\n' "$path"
curl -k -L --fail --silent --show-error "$base/$path" \
| rg -n -C 5 'splice|appendChild|insertBefore|removeChild|domNode|parent' \
| head -240 || true
doneRepository: FlowingCode/ChatAssistant
Length of output: 4126
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- TreeChangeProcessor splice implementation ---'
curl -k -L --fail --silent --show-error \
https://raw.githubusercontent.com/vaadin/flow/master/flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java \
| sed -n '150,255p'
printf '%s\n' '--- client source paths related to node lists and DOM features ---'
curl -k -L --fail --silent --show-error \
'https://api.github.com/repos/vaadin/flow/git/trees/master?recursive=1' \
| jq -r '.tree[].path' \
| rg 'flow-client/.+(NodeList|Dom|Element|Feature|StateNode|TreeChange)' \
| head -200Repository: FlowingCode/ChatAssistant
Length of output: 4345
🌐 Web query:
site:github.com/vaadin/flow/blob/master/flow-client/src/main/java/com/vaadin/client/flow NodeList.java
💡 Result:
There is no standard Java library class named NodeList.java. In the context of Java development, NodeList typically refers to the org.w3c.dom.NodeList interface, which is a standard part of the Java Platform (specifically the Java XML API) used to represent an ordered collection of nodes [1]. In other contexts, such as the Vaadin framework, NodeList is a custom class (e.g., com.vaadin.client.flow.nodefeature.NodeList) used to manage state nodes within that specific framework's architecture [1]. Because NodeList is a common name, it is frequently used by various third-party libraries for their own internal collections or tree-management structures. If you are encountering a file named NodeList.java in a project, it is almost certainly a custom implementation specific to that codebase or its dependencies.
Citations:
🌐 Web query:
"class NodeList" "com.vaadin.client.flow" splice
💡 Result:
The class com.vaadin.flow.internal.nodefeature.NodeList in Vaadin Flow does not contain a "splice" method [1][2][3]. NodeList is an internal API in Vaadin used for managing state node features structured as lists, and it is intended for internal framework use only [1][2]. It provides standard list-like operations such as add, addAll, remove, and get, but it does not implement or expose a splice-style method [1][2][3]. It is important to distinguish this Java class from the browser's DOM NodeList interface. In JavaScript, a DOM NodeList is an array-like object that also does not have a native splice method; developers often convert it to an Array if they need to use splice [4]. If you are working with Vaadin Flow and attempting to manipulate element children or other node features, you should use the provided API methods such as remove(int index) or insertChild/setChild/appendChild as available on the relevant components or nodes [5][6]. If you found a reference to "splice" in a Vaadin-related context, it is likely unrelated to this internal Java class and may be a misunderstanding of how browser-side JavaScript interacts with DOM elements [4].
Citations:
- 1: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/NodeList.html
- 2: https://origin.vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/NodeList.html
- 3: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/14.14.0/com/vaadin/flow/internal/nodefeature/NodeList.html
- 4: https://stackoverflow.com/questions/36346892/how-to-let-splice-function-apply-to-nodelist-array
- 5: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 6: https://origin.vaadin.com/api/platform/current/com/vaadin/flow/dom/Node.html
🏁 Script executed:
#!/bin/bash
set -e
for path in \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/NodeList.java \
flow-client/src/main/java/com/vaadin/client/flow/NodeList.java \
flow-client/src/main/java/com/vaadin/client/flow/StateNode.java
do
printf '\n--- %s ---\n' "$path"
curl -k -L --retry 2 --fail --silent --show-error \
"https://raw.githubusercontent.com/vaadin/flow/24.8.0/$path" \
| sed -n '1,280p' \
| rg -n -C 8 'class NodeList|void splice|splice\(|appendChild|insertBefore|removeChild|getDomNode|domNode' \
| head -300 || true
doneRepository: FlowingCode/ChatAssistant
Length of output: 6245
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- TreeChangeProcessor imports and DOM list listener ---'
curl -k -L --fail --silent --show-error \
https://raw.githubusercontent.com/vaadin/flow/24.8.0/flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java \
| sed -n '1,180p'
printf '%s\n' '--- candidate client node-feature sources ---'
for path in \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/ElementChildrenList.java \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/NodeFeature.java \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/NodeList.java \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/NodeListChangeListener.java \
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/ListSpliceEvent.java
do
printf '\n--- %s ---\n' "$path"
curl -k -L --fail --silent --show-error \
"https://raw.githubusercontent.com/vaadin/flow/24.8.0/$path" \
| rg -n -C 8 'class |onSplice|splice|appendChild|insertBefore|removeChild|domNode|parentNode' \
| head -260 || true
doneRepository: FlowingCode/ChatAssistant
Length of output: 12399
🌐 Web query:
"ElementChildrenList" "com.vaadin.client.flow" Vaadin
💡 Result:
ElementChildrenList is an internal class in the Vaadin Flow framework, specifically located in the package com.vaadin.flow.internal.nodefeature [1][2]. It is designed to maintain a list of nodes that describe the child elements of a given element within the internal state tree [1][3]. Key characteristics of ElementChildrenList include: Internal Use Only: The class is explicitly marked for internal use only and is subject to being renamed or removed in future releases [1][2]. It is not intended for use by application developers. Functionality: It extends StateNodeNodeList [1][3] and provides methods to manage the hierarchical structure of elements, such as adding, removing, retrieving, and clearing child nodes [1][4]. Role in Vaadin Flow: Vaadin Flow maintains a tree of state nodes on the server that is synchronized with the DOM tree in the browser [5]. ElementChildrenList serves as a specialized feature of a StateNode, allowing the framework to track and manipulate the child elements of a component [1][6]. Regarding your query about com.vaadin.client.flow, this package exists in the client-side module of Vaadin Flow (often found in the flow-client library) [7][8]. While ElementChildrenList is a server-side internal class (as indicated by the com.vaadin.flow.internal package structure), the client-side libraries handle the interpretation of state changes and the subsequent updates to the browser's DOM [9][10][5]. Developers interact with these structures indirectly through the high-level Vaadin component APIs rather than accessing these internal classes directly [5].
Citations:
- 1: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 2: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/14.14.0/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 3: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/StateNodeNodeList.html
- 4: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/14.10.3/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 5: https://vaadin.com/docs/latest/flow/what-is-flow
- 6: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/NodeFeatures.html
- 7: https://javadoc.io/static/com.vaadin/flow-client/1.0.9/overview-summary.html
- 8: https://javadoc.io/doc/com.vaadin/flow-client/24.4.6/index.html
- 9: https://github.com/vaadin/flow/blob/master/flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java
- 10: https://github.com/vaadin/flow/blob/master/flow-client/src/main/java/com/vaadin/client/communication/MessageHandler.java
🌐 Web query:
"ElementChildrenList" vaadin flow-client source
💡 Result:
In Vaadin Flow, ElementChildrenList is an internal class located in the com.vaadin.flow.internal.nodefeature package [1][2][3]. It serves as a node feature responsible for managing and describing the child elements of a UI component's state node [1][3]. Key characteristics of ElementChildrenList include: Purpose: It acts as a list that holds references to StateNode objects, specifically representing the child elements associated with a parent element [1][2][3]. Internal Status: The class is explicitly marked for internal use only [1][3]. Vaadin documentation notes that its API may be renamed or removed in future releases [1][3]. Inheritance: It extends StateNodeNodeList, which is a specialized list structure designed to contain StateNode instances [2][3]. Integration: It is identified by the constant ELEMENT_CHILDREN in the NodeFeatures class, which is used to look up or reference this feature [4]. Because it is an internal implementation detail of the Vaadin Flow framework, developers are generally discouraged from using it directly in application code, as changes to the framework may break such dependencies [1][3]. Standard component and element manipulation should be performed through the public Element API rather than interacting with node features directly.
Citations:
- 1: https://javadoc.io/static/com.vaadin/vaadin-platform-javadoc/14.10.6/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 2: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/StateNodeNodeList.html
- 3: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/ElementChildrenList.html
- 4: https://vaadin.com/api/platform/current/com/vaadin/flow/internal/nodefeature/NodeFeatures.html
🏁 Script executed:
#!/bin/bash
set -e
paths=(
flow-client/src/main/java/com/vaadin/client/flow/element/ElementChildrenList.java
flow-client/src/main/java/com/vaadin/client/flow/nodefeature/ElementChildrenList.java
flow-client/src/main/java/com/vaadin/client/flow/NodeListChangeListener.java
flow-client/src/main/java/com/vaadin/client/flow/NodeFeature.java
flow-client/src/main/java/com/vaadin/client/flow/StateNode.java
)
for path in "${paths[@]}"; do
url="https://raw.githubusercontent.com/vaadin/flow/24.8.0/$path"
code=$(curl -k -L --silent --output /tmp/flow-source --write-out '%{http_code}' "$url" || true)
printf '\n--- %s [%s] ---\n' "$path" "$code"
if [ "$code" = 200 ]; then
rg -n -C 10 'class |onSplice|splice|appendChild|insertBefore|removeChild|getDomNode|parentNode' /tmp/flow-source | head -320
fi
doneRepository: FlowingCode/ChatAssistant
Length of output: 2580
🏁 Script executed:
#!/bin/bash
set -e
tmp=$(mktemp -d)
url=https://repo1.maven.org/maven2/com/vaadin/flow-client/24.8.0/flow-client-24.8.0-sources.jar
curl -k -L --fail --silent --show-error "$url" -o "$tmp/flow-client-sources.jar"
printf '%s\n' '--- matching source entries ---'
unzip -Z1 "$tmp/flow-client-sources.jar" \
| rg 'NodeList|ElementChildren|TreeChange|StateNode|Dom' \
| head -120
printf '%s\n' '--- DOM/list listener source ---'
for entry in $(unzip -Z1 "$tmp/flow-client-sources.jar" | rg 'NodeList|ElementChildren|TreeChange|StateNode'); do
if unzip -p "$tmp/flow-client-sources.jar" "$entry" 2>/dev/null \
| rg -q 'appendChild|insertBefore|removeChild|onSplice|ListSplice'; then
printf '\n--- %s ---\n' "$entry"
unzip -p "$tmp/flow-client-sources.jar" "$entry" \
| rg -n -C 12 'appendChild|insertBefore|removeChild|onSplice|ListSplice|domNode' \
| head -260
fi
doneRepository: FlowingCode/ChatAssistant
Length of output: 5157
🏁 Script executed:
#!/bin/bash
set -e
tmp=$(mktemp -d)
curl -k -L --fail --silent --show-error \
https://repo1.maven.org/maven2/com/vaadin/flow-client/24.8.0/flow-client-24.8.0-sources.jar \
-o "$tmp/flow-client-sources.jar"
for entry in \
com/vaadin/client/flow/dom/DomApi.java \
com/vaadin/client/flow/dom/DomApiImpl.java \
com/vaadin/client/flow/dom/DomElement.java \
com/vaadin/client/flow/dom/DomNode.java \
com/vaadin/client/flow/dom/PolymerDomApiImpl.java \
com/vaadin/client/flow/StateNode.java
do
printf '\n--- %s ---\n' "$entry"
unzip -p "$tmp/flow-client-sources.jar" "$entry" \
| rg -n -C 10 'appendChild|insertBefore|removeChild|splice|addSplice|DomApi|DomNode|children|parent' \
| head -360
doneRepository: FlowingCode/ChatAssistant
Length of output: 12186
🏁 Script executed:
#!/bin/bash
set -e
tmp=$(mktemp -d)
curl -k -L --fail --silent --show-error \
https://repo1.maven.org/maven2/com/vaadin/flow-client/24.8.0/flow-client-24.8.0-sources.jar \
-o "$tmp/flow-client-sources.jar"
printf '%s\n' '--- all client references to addSpliceListener and DOM child operations ---'
for entry in $(unzip -Z1 "$tmp/flow-client-sources.jar" | grep '\.java$'); do
text=$(unzip -p "$tmp/flow-client-sources.jar" "$entry")
if printf '%s' "$text" | rg -q 'addSpliceListener|appendChild|insertBefore|removeChild'; then
printf '\n--- %s ---\n' "$entry"
printf '%s' "$text" | rg -n -C 14 'addSpliceListener|appendChild|insertBefore|removeChild' | head -320
fi
doneRepository: FlowingCode/ChatAssistant
Length of output: 21345
🏁 Script executed:
#!/bin/bash
set -e
tmp=$(mktemp -d)
curl -k -L --fail --silent --show-error \
https://repo1.maven.org/maven2/com/vaadin/flow-client/24.8.0/flow-client-24.8.0-sources.jar \
-o "$tmp/flow-client-sources.jar"
unzip -p "$tmp/flow-client-sources.jar" \
com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java \
| sed -n '1025,1165p'Repository: FlowingCode/ChatAssistant
Length of output: 5942
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
class Node {
constructor(name) { this.name = name; this.parentNode = null; this.children = []; }
appendChild(child) {
if (child.parentNode) child.parentNode.removeChild(child);
child.parentNode = this;
this.children.push(child);
}
insertBefore(child, ref) {
if (ref !== null && ref.parentNode !== this) {
throw new Error(`NotFoundError: ${ref.name} is not a child of ${this.name}`);
}
if (child.parentNode) child.parentNode.removeChild(child);
child.parentNode = this;
const index = ref === null ? this.children.length : this.children.indexOf(ref);
this.children.splice(index, 0, child);
}
removeChild(child) {
const index = this.children.indexOf(child);
if (index < 0) throw new Error(`NotFoundError: ${child.name}`);
this.children.splice(index, 1);
child.parentNode = null;
}
nextSibling() {
if (!this.parentNode) return null;
const siblings = this.parentNode.children;
const i = siblings.indexOf(this);
return i < 0 || i + 1 === siblings.length ? null : siblings[i + 1];
}
}
function flowAddChildren(root, stateChildren, index, added) {
// Equivalent to SimpleElementBindingStrategy.addChildren() for a nonzero index.
const previous = stateChildren[index - 1];
const beforeRef = previous ? previous.nextSibling() : null;
for (const child of added) {
root.insertBefore(child, beforeRef);
}
}
const body = new Node('body');
const root = new Node('animated-fab');
const first = new Node('chatWindow');
const fab = new Node('fabWrapper');
const last = new Node('mobileChatWindow');
const bodySibling = new Node('bodySibling');
root.appendChild(first);
root.appendChild(fab);
root.appendChild(last);
body.appendChild(root);
// The portal operation from fcChatAssistantPortalFab().
body.appendChild(fab);
// Inserting at the position immediately after the portaled StateNode uses the
// portaled node's body sibling as beforeRef, not a child of root.
const inserted = new Node('insertedChild');
let insertionError = null;
try {
flowAddChildren(root, [first, fab, last], 2, [inserted]);
} catch (e) {
insertionError = e.message;
}
// Removing a different child follows the framework's parent check and is safe.
let removeOtherChildSafe = false;
if (last.parentNode === root) {
root.removeChild(last);
removeOtherChildSafe = true;
}
// The project's teardown restore logic returns the portaled node to its home.
root.appendChild(last);
const home = root;
if (fab.parentNode === body) home.appendChild(fab);
const restored = fab.parentNode === root;
console.log(JSON.stringify({ insertionError, removeOtherChildSafe, restored }));
JSRepository: FlowingCode/ChatAssistant
Length of output: 231
🏁 Script executed:
#!/bin/bash
set -e
tmp=$(mktemp -d)
curl -k -L --fail --silent --show-error \
https://repo1.maven.org/maven2/com/vaadin/flow-client/24.8.0/flow-client-24.8.0-sources.jar \
-o "$tmp/flow-client-sources.jar"
unzip -p "$tmp/flow-client-sources.jar" \
com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java \
| rg -n -C 16 'getPreviousSibling|getNextSibling' | head -220Repository: FlowingCode/ChatAssistant
Length of output: 3648
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
class Node {
constructor(name) { this.name = name; this.parentNode = null; this.children = []; }
appendChild(child) {
if (child.parentNode) child.parentNode.removeChild(child);
child.parentNode = this;
this.children.push(child);
}
insertBefore(child, ref) {
if (ref !== null && ref.parentNode !== this) {
throw new Error(`NotFoundError: ${ref.name} is not a child of ${this.name}`);
}
if (child.parentNode) child.parentNode.removeChild(child);
child.parentNode = this;
const index = ref === null ? this.children.length : this.children.indexOf(ref);
this.children.splice(index, 0, child);
}
removeChild(child) {
const index = this.children.indexOf(child);
if (index < 0) throw new Error(`NotFoundError: ${child.name}`);
this.children.splice(index, 1);
child.parentNode = null;
}
nextSibling() {
if (!this.parentNode) return null;
const i = this.parentNode.children.indexOf(this);
return i >= 0 && i + 1 < this.parentNode.children.length
? this.parentNode.children[i + 1] : null;
}
}
function flowBeforeRef(root, stateChildren, index) {
const previous = index > 0 ? stateChildren[index - 1] : null;
return previous ? previous.nextSibling() : null;
}
const body = new Node('body');
const root = new Node('animated-fab');
const first = new Node('chatWindow');
const fab = new Node('fabWrapper');
const last = new Node('mobileChatWindow');
const bodySibling = new Node('bodySibling');
root.appendChild(first);
root.appendChild(last);
body.appendChild(root);
body.appendChild(fab); // portal
body.appendChild(bodySibling); // unrelated body node after the portal
const stateChildren = [first, fab, last];
const beforeRef = flowBeforeRef(root, stateChildren, 2);
let error = null;
try { root.insertBefore(new Node('insertedChild'), beforeRef); }
catch (e) { error = e.message; }
console.log(JSON.stringify({
beforeRef: beforeRef && beforeRef.name,
beforeRefParent: beforeRef && beforeRef.parentNode && beforeRef.parentNode.name,
error
}));
JSRepository: FlowingCode/ChatAssistant
Length of output: 284
Keep fabWrapper in the Flow parent.
When Flow inserts a child after the portaled fabWrapper, addChildren can pass a <body> sibling to <animated-fab>.insertBefore, which raises NotFoundError. Keep the wrapper under its Flow parent or handle portal-aware insertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`
around lines 39 - 54, Update fcChatAssistantPortalFab so fabWrapper remains
under its Flow parent instead of being appended to document.body; adjust the
anchoring behavior to avoid disrupting Flow’s sibling insertion and preserve
correct fixed positioning, or otherwise make Flow’s addChildren/insertion path
portal-aware so body-level siblings are never passed to
animated-fab.insertBefore.
| const notify = (rect) => { | ||
| // Ignore the pre-layout 0x0 state so the initial delivery reflects the real size. | ||
| if (rect.width === 0 && rect.height === 0) { | ||
| return; | ||
| } | ||
| const above = isAbove(rect); | ||
| if (above !== entry.last) { | ||
| entry.last = above; | ||
| root.$server?.onScreenSizeChange(key, above); | ||
| } | ||
| }; | ||
|
|
||
| entry.observer = new ResizeObserver((entries) => notify(entries[0].contentRect)); | ||
| entry.observer.observe(overlayDiv); | ||
| root.__fcScreenSizeListeners[key] = entry; | ||
| // Deliver the current state immediately (if already laid out; otherwise the observer's first | ||
| // callback delivers it). | ||
| notify(overlayDiv.getBoundingClientRect()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Measure the same box in the initial delivery and in the observer callback.
notify receives entries[0].contentRect from the ResizeObserver at Line 343, but overlayDiv.getBoundingClientRect() at Line 348. contentRect excludes padding and border; getBoundingClientRect() includes them. The observed element is container, a VerticalLayout that ChatAssistant configures with setPadding(true) in desktop mode. The two measurements therefore differ by the padding, so a threshold close to the current size can report one state on registration and the opposite state on the first observer callback.
🐛 Proposed fix
entry.observer = new ResizeObserver((entries) => notify(entries[0].contentRect));
entry.observer.observe(overlayDiv);
root.__fcScreenSizeListeners[key] = entry;
// Deliver the current state immediately (if already laid out; otherwise the observer's first
// callback delivers it).
- notify(overlayDiv.getBoundingClientRect());
+ // Use the content box so it matches the ResizeObserver's contentRect.
+ const style = window.getComputedStyle(overlayDiv);
+ notify({
+ width: overlayDiv.clientWidth
+ - parseFloat(style.paddingLeft) - parseFloat(style.paddingRight),
+ height: overlayDiv.clientHeight
+ - parseFloat(style.paddingTop) - parseFloat(style.paddingBottom),
+ });📝 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.
| const notify = (rect) => { | |
| // Ignore the pre-layout 0x0 state so the initial delivery reflects the real size. | |
| if (rect.width === 0 && rect.height === 0) { | |
| return; | |
| } | |
| const above = isAbove(rect); | |
| if (above !== entry.last) { | |
| entry.last = above; | |
| root.$server?.onScreenSizeChange(key, above); | |
| } | |
| }; | |
| entry.observer = new ResizeObserver((entries) => notify(entries[0].contentRect)); | |
| entry.observer.observe(overlayDiv); | |
| root.__fcScreenSizeListeners[key] = entry; | |
| // Deliver the current state immediately (if already laid out; otherwise the observer's first | |
| // callback delivers it). | |
| notify(overlayDiv.getBoundingClientRect()); | |
| const notify = (rect) => { | |
| // Ignore the pre-layout 0x0 state so the initial delivery reflects the real size. | |
| if (rect.width === 0 && rect.height === 0) { | |
| return; | |
| } | |
| const above = isAbove(rect); | |
| if (above !== entry.last) { | |
| entry.last = above; | |
| root.$server?.onScreenSizeChange(key, above); | |
| } | |
| }; | |
| entry.observer = new ResizeObserver((entries) => notify(entries[0].contentRect)); | |
| entry.observer.observe(overlayDiv); | |
| root.__fcScreenSizeListeners[key] = entry; | |
| // Deliver the current state immediately (if already laid out; otherwise the observer's first | |
| // callback delivers it). | |
| // Use the content box so it matches the ResizeObserver's contentRect. | |
| const style = window.getComputedStyle(overlayDiv); | |
| notify({ | |
| width: overlayDiv.clientWidth | |
| - parseFloat(style.paddingLeft) - parseFloat(style.paddingRight), | |
| height: overlayDiv.clientHeight | |
| - parseFloat(style.paddingTop) - parseFloat(style.paddingBottom), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/META-INF/resources/frontend/fc-chat-assistant-movement.js`
around lines 331 - 348, Use the same box measurement for both initial and
observer deliveries in the notify flow around entry.observer and overlayDiv:
observe and measure overlayDiv consistently, or consistently use its content
box, so padding and border are treated identically. Replace the mismatched
getBoundingClientRect() initial measurement with the measurement corresponding
to the ResizeObserver callback while preserving the existing zero-size and
threshold logic.
| vaadin-popover-overlay::part(overlay) { | ||
| /* Cross-theme fallback: Lumo radius token, then Aura base radius, then a literal. */ | ||
| border-radius: var(--lumo-border-radius-l, var(--aura-base-radius, 12px)); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Scope the overlay border-radius rule to the chat assistant.
vaadin-popover-overlay::part(overlay) carries no fc-chat-assistant-popover class. This file is applied through @CssImport, so the rule enters the host application's document scope and restyles every vaadin-popover-overlay in the application, not only the chat window. Every other rule in this file is scoped with the add-on class (see Lines 55-56 and Lines 68-69). Add the class to keep the add-on's styles contained.
🐛 Proposed fix
-vaadin-popover-overlay::part(overlay) {
+vaadin-popover-overlay.fc-chat-assistant-popover::part(overlay),
+vaadin-popover.fc-chat-assistant-popover::part(overlay) {
/* Cross-theme fallback: Lumo radius token, then Aura base radius, then a literal. */
border-radius: var(--lumo-border-radius-l, var(--aura-base-radius, 12px));
}📝 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.
| vaadin-popover-overlay::part(overlay) { | |
| /* Cross-theme fallback: Lumo radius token, then Aura base radius, then a literal. */ | |
| border-radius: var(--lumo-border-radius-l, var(--aura-base-radius, 12px)); | |
| } | |
| vaadin-popover-overlay.fc-chat-assistant-popover::part(overlay), | |
| vaadin-popover.fc-chat-assistant-popover::part(overlay) { | |
| /* Cross-theme fallback: Lumo radius token, then Aura base radius, then a literal. */ | |
| border-radius: var(--lumo-border-radius-l, var(--aura-base-radius, 12px)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/resources/META-INF/resources/frontend/styles/fc-chat-assistant-style.css`
around lines 45 - 48, Scope the `vaadin-popover-overlay::part(overlay)`
border-radius selector to the `fc-chat-assistant-popover` class, matching the
containment pattern used by the other rules in this stylesheet. Keep the
existing fallback radius values unchanged.
|
3 participants
Close #47 #64 #67
Chat Assistant Features (5.1.0-SNAPSHOT)
FAB (Floating Action Button)
Icon
setFabIcon(...), with a built-in chatbot icon used by default.Sizing
addFabThemeVariants(LUMO_SMALL, LUMO_LARGE)resizes the FAB (and its icon).LUMO_SUCCESS,LUMO_ERROR,LUMO_CONTRAST) restyle the FAB, with the icon automatically matching the button color.Positioning
setFabPosition(FabPosition)places the FAB in any of the four screen corners.resetFabPosition()restores the default position.Dragging
setFabMovable(...)allows the FAB to be dragged by the user.Bounded Placement
setFabAnchoredToViewport(false)positions the FAB inside a container instead of floating over the viewport.Unread badge colors
setUnreadBadgeColors(background, color)to change the badge background and text colors.Chat Window
Resizing
setWindowResizable(...)enables resizing using eight drag handles.setResizeIndicatorsVisible(...), showing arrows that indicate each handle's drag direction.Sizing & Bounds
setWindowWidth(...)setWindowHeight(...)setWindowMinWidth(...)setWindowMaxWidth(...)setWindowMinHeight(...)setWindowMaxHeight(...)Responsive / Mobile Mode
Modes
setMode(...)/setMobileMode(...)Auto-Switching
mobileBreakpoint.Listeners
addModeChangedListener(...)for mobile/desktop mode changes.addScreenSizeListener(...)for detecting when the chat window crosses a configured size threshold.Construction & Miscellaneous
Builder API
ChatAssistant.builder()provides a declarative configuration API.Native Markdown
markdown-editordependency has been removed.Summary by CodeRabbit
New Features
Documentation
Chores