fix: use grid instead of virtual-list #48
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughReplaced VirtualList-based message rendering with a Grid-based implementation in ChatAssistant.java, added a per-column content renderer, applied Lumo no-border row theme and a compact CSS class, and added a CSS rule to remove grid cell padding. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css (1)
58-61: Use ::part() to style Grid internals; current selector likely won’t pierce the shadow DOM
vaadin-grid-cell-contentlives inside the Grid’s shadow root. Global CSS selectors won’t reach it. Prefer ::part selectors, which are supported by Grid, to actually zero out padding.Apply this diff:
-vaadin-grid.chat-assistant-grid-compact vaadin-grid-cell-content { - padding: 0px; -} +vaadin-grid.chat-assistant-grid-compact::part(cell) { + padding: 0; +} +/* Optional: if a header row is visible, keep it compact as well */ +vaadin-grid.chat-assistant-grid-compact::part(header-cell) { + padding: 0; +}If you still see residual spacing, also consider making rows compact via theme variant (see Java comment) instead of relying solely on CSS.
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (4)
74-78: Keep a direct reference to the content column to avoid “magic key” lookupsLater you fetch the column by key "content". That’s brittle and can return null if the key changes. Store the column once and reuse it.
Apply this diff:
- private Grid<T> content = new Grid<>(); + private Grid<T> content = new Grid<>(); + private Grid.Column<T> contentColumn;
151-158: Tighten Grid setup: add compact/wrap variants, disable selection, and use a pure renderer column
- Add LUMO_COMPACT and LUMO_WRAP_CELL_CONTENT variants to reduce paddings and ensure long markdown wraps without horizontal scrollbars.
- Disable row selection to prevent accidental selection highlight on message clicks.
- Use
addColumn(new ComponentRenderer<>(...))directly instead of providing a value provider and then overriding it with a renderer. The value provider is unused and can affect sort defaults.Apply this diff:
- content.addThemeVariants(GridVariant.LUMO_NO_BORDER); - content.addThemeVariants(GridVariant.LUMO_NO_ROW_BORDERS); - content.setClassName("chat-assistant-grid-compact"); - content.addColumn(m->m.getContent()).setKey("content").setRenderer(new ComponentRenderer<>(message -> new ChatMessage<T>(message, markdownEnabled), + content.addThemeVariants( + GridVariant.LUMO_NO_BORDER, + GridVariant.LUMO_NO_ROW_BORDERS, + GridVariant.LUMO_COMPACT, + GridVariant.LUMO_WRAP_CELL_CONTENT); + content.setSelectionMode(Grid.SelectionMode.NONE); + content.setClassName("chat-assistant-grid-compact"); + contentColumn = content.addColumn(new ComponentRenderer<>( + message -> new ChatMessage<T>(message, markdownEnabled), (component, message) -> { ((ChatMessage<T>) component).setMessage(message); return component; - })); + })) + .setKey("content");Follow-up check (manual): with many long markdown messages and lazy loading enabled, scroll up/down rapidly and confirm:
- No duplicate or missing rows.
- No selection highlight flash on click/tap.
- Lines wrap as expected without horizontal scrolling.
384-387: Avoid stringly-typed column lookup; use the stored referenceThis prevents an NPE if the key is changed elsewhere and is marginally faster.
Apply this diff:
- content.getColumnByKey("content").setRenderer(renderer); + contentColumn.setRenderer(renderer);
207-213: Update stale Javadoc: it references VirtualList but we now use GridMinor but could confuse users of the API.
Apply this diff:
- /** - * Sets the data provider of the internal VirtualList. - * - * @param dataProvider the data provider to be used - */ + /** + * Sets the data provider of the internal Grid. + * + * @param dataProvider the data provider to be used + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java(4 hunks)src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css:20-28
Timestamp: 2025-07-22T20:00:43.518Z
Learning: In ChatAssistant's CSS (src/main/resources/META-INF/frontend/styles/chat-assistant-styles.css), the double 180-degree rotation trick on both .chat-assistant-resizable-vertical-layout and .chat-assistant-container-vertical-layout is used to move the resize handle from the default bottom-right corner to the upper-left corner. This positioning is more suitable for resizing the chat window since the chat bubble is positioned in the bottom-right part of the viewport.
📚 Learning: 2025-07-22T19:52:28.019Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-22T19:52:28.019Z
Learning: When using ReactAdapterComponent with custom React components in ChatAssistant, standard Vaadin component hierarchy methods like add() cannot be used because the component doesn't implement HasComponents, and the DOM structure is controlled by React. Direct DOM manipulation with executeJs may be necessary to integrate Vaadin components (like Avatar) into React-generated DOM structures.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
📚 Learning: 2025-07-24T13:40:40.471Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-24T13:40:40.471Z
Learning: In ChatAssistant's initializeAvatar method (src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java), direct DOM manipulation using executeJs is intentionally used to move the Avatar component to be a child of a button that is inside a div within the React-controlled web component structure. This approach is necessary because the target elements are created by React, and standard Vaadin component hierarchy methods cannot be used due to ReactAdapterComponent limitations.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
📚 Learning: 2025-07-24T13:41:19.117Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/resources/META-INF/frontend/react/animated-fab.tsx:0-0
Timestamp: 2025-07-24T13:41:19.117Z
Learning: In the ChatAssistant animated-fab React component (src/main/resources/META-INF/frontend/react/animated-fab.tsx), the Floating Action Button (FAB) appears empty in the React code but gets its icon content from the Avatar Vaadin component, which is inserted via DOM manipulation from the Java backend's initializeAvatar method.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
🔇 Additional comments (1)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (1)
32-34: LGTM: correct switch to Grid APIImports for Grid and GridVariant look right for the new implementation. No issues spotted here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #47
Summary by CodeRabbit