Skip to content

feat: implement row actions for EasyGrid#16

Open
javier-godoy wants to merge 4 commits into
masterfrom
impl/row-actions
Open

feat: implement row actions for EasyGrid#16
javier-godoy wants to merge 4 commits into
masterfrom
impl/row-actions

Conversation

@javier-godoy

@javier-godoy javier-godoy commented Jun 24, 2026

Copy link
Copy Markdown
Member

Pending issues

  • Dynamic and localizable confirmation dialogs (Deferred — future feature)

    • Add per-row dynamic message variants using ValueProvider<T, String>
    • Implement localizable defaults for "Ok" / "Cancel" button labels via i18n hook or I18NProvider
    • Consider adding full-control variant: withConfirmation(SerializableSupplier)
  • Menu renderers (dropdown/context menu) don't support tooltips and styling

    • Forward tooltip to each MenuItem via setTooltipText() or title attribute
    • Copy CSS classes from action element to each menu item
    • Copy inline styles from action element to each menu item
    • Unresolved: Theme variants (ButtonVariant) cannot map to menu items — pending decision on whether to treat them as inline-only with a spec caveat, or design a menu-item mapping strategy

Summary by CodeRabbit

Summary

  • New Features
    • Added EasyGrid row actions with three styles: inline buttons, dropdown overflow, and context menu.
    • Added a new row-actions API (including icons via icon factories/value providers), tooltips, confirmations, and per-row conditional visibility/enabled state.
    • Added row-actions customization with style-driven renderer switching, default button variants, and a refreshRowActions hook.
  • Bug Fixes
    • Improved renderer switching/cleanup and ensured theme/variant updates require an explicit refresh.
  • Documentation
    • Updated the row-actions feature spec and examples to the new style-driven API.
  • Tests
    • Added unit and end-to-end coverage for menus, predicates, confirmations, dynamic icons, theme variants, and renderer switching.
  • Chores
    • Updated dependency versions and test/build tooling.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Implements EasyGrid row actions with new APIs, renderer strategies, frontend assets, demos, tests, and Maven updates. The row-actions feature document is rewritten to match the implemented API.

Changes

EasyGrid Row Actions

Layer / File(s) Summary
Docs and public contracts
FEATURE_ROW_ACTIONS.md, src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/*.java, src/main/java/com/flowingcode/vaadin/addons/easygrid/EasyGridWrapper.java
The feature document and the public row-actions contracts are updated to describe the implemented row-actions surface, and the wrapper now exposes the row-actions manager.
Row-action model and Lit builder
src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java, src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java
EasyRowAction adds row-level behavior, execution, and rendering hooks, and LitRendererBuilder adds Lit template assembly, bindings, events, spreading, and escaping.
Renderers and frontend assets
src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/*Renderer.java, src/main/resources/META-INF/frontend/fc-icon.ts, src/main/resources/META-INF/frontend/fc-dynamic-buttons.css, src/main/resources/META-INF/frontend/styles/static_addon_styles, src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsManager.java, src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsStyle.java
The frontend icon and button assets are added, and the inline, context-menu, and dropdown renderer implementations are introduced together with the manager and style switching logic.
Demos and build setup
pom.xml, src/test/java/com/flowingcode/vaadin/addons/easygrid/**/*Demo.java
The Maven configuration is updated, and the demo views add row-action examples to the demo app.
Unit and template tests
src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/*Test.java, src/test/java/com/flowingcode/vaadin/addons/easygrid/test/EasyRowActionUnitTest.java
The unit tests cover action rendering, execution guards, theme output, builder behavior, and actions-column lifecycle across renderer modes.
Integration harness wiring
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/*, src/test/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener
The shared TestBench setup, RPC contract, instrumented view, remote action wrapper, and service registration are added for row-action integration tests.
End-to-end interaction coverage
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
The browser tests exercise inline, context-menu, and dropdown row actions, renderer switching, confirmation flows, and theme refresh behavior.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • FlowingCode/EasyGridAddon#11: Updates the same FEATURE_ROW_ACTIONS.md API surface around visibleWhen, enabledWhen, tooltip, and expanded addRowAction overloads.
  • FlowingCode/EasyGridAddon#14: Also documents the row-actions API changes around setDefaultRowActionVariants, setRowActionsRenderer, refreshRowActions, and removal behavior.

Suggested reviewers

  • scardanzan
  • paodb
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.21% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding row actions support to EasyGrid.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch impl/row-actions

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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 `@pom.xml`:
- Around line 73-77: The json-migration-helper dependency is currently part of
the main build graph even though it is only used by test classes like
EasyRowActionITView and ViewInitializerImpl. Update the dependency entry in
pom.xml to add test scope so it is only available during tests and does not leak
into the published add-on or its transitive dependencies.

In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java`:
- Around line 54-75: The DropdownMenuRowActionsRenderer is calling Vaadin’s
private context-menu connector through
grid.$contextMenuTargetConnector.openOnHandler and removeListener(), which is
version-sensitive. Update the handling in createContextMenu() and the row-action
handler setup to use the public Grid/GridContextMenu APIs for opening and
managing the context menu, and remove any direct executeJs access to the
internal connector.

In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java`:
- Around line 301-304: The EasyRowAction title handling is stripping manually
set title attributes during refresh. Update the attribute forwarding in
EasyRowAction so refreshRowActions() preserves a caller-provided
getElement().setAttribute("title", ...) value unless an explicit tooltip
provider is being used, and keep the per-row bind("title", tooltipProvider)
behavior in sync with that logic. Use the copyAllAttributesAndPropertiesExcept
and bind("title", tooltipProvider) paths to ensure the title field supports both
manual attributes/properties and dynamic tooltips.
- Around line 258-265: The confirm flow in EasyRowAction.execute() bypasses the
server-side row guard by calling actionHandler.accept(item) directly from the
ConfirmDialog confirm listener. Update the confirm listener to re-check the same
visibleWhen/enabledWhen condition used before opening the dialog, and only
invoke the action when the row still passes the guard. Use the existing
execute(), confirmDialogSupplier, and actionHandler logic as the place to apply
this check.

In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java`:
- Around line 66-82: The semaphore in AbstractViewTest.setup() is being released
too early, so the browser limit is not enforced during the actual test
execution. Move the permit release out of setup and hold it for the full
lifecycle by pairing semaphore.acquire() with a release in a teardown method or
equivalent final cleanup path, ensuring the ChromeDriver/TestBench driver
created in setup remains counted until the test finishes.

In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java`:
- Around line 243-250: The test in EasyRowActionIT opens a second context menu
immediately after sending ESCAPE without waiting for the first overlay to
disappear, which makes the interaction flaky. Update the sequence around the
first grid.getCell(...).contextClick() so the menu is closed through the
existing closeMenu() helper before opening the next row’s context menu. Keep the
fix localized to the context-menu assertions in EasyRowActionIT and reuse the
helper rather than relying on a raw ESCAPE keypress alone.

In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java`:
- Around line 54-58: The demo comment and behavior around
RowActionsDynamicDemo#visibleWhen are inconsistent with the tested contract:
fluent mutators should update the row action state without requiring a separate
refresh. Update the demo to match the API behavior enforced by EasyRowActionIT
by either removing the note about needing refreshRowActions() or adding the
explicit refresh call after changing visibleWhen. Keep the example centered on
editAction and the Checkbox listener so the demo reflects the same
self-refreshing behavior as the tests.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a18e1ed-ad7a-4f1e-accd-a4f326f91b59

📥 Commits

Reviewing files that changed from the base of the PR and between 2877880 and 178826b.

📒 Files selected for processing (31)
  • FEATURE_ROW_ACTIONS.md
  • pom.xml
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/EasyGridWrapper.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/Constant.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/ContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/HasRowActions.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsManager.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsStyle.java
  • src/main/resources/META-INF/frontend/fc-dynamic-buttons.css
  • src/main/resources/META-INF/frontend/fc-icon.ts
  • src/main/resources/META-INF/frontend/styles/static_addon_styles
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/EasyGridDemoView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsMenuDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilderTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITCallables.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/RmiEasyRowAction.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/ViewInitializerImpl.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/test/EasyRowActionUnitTest.java
  • src/test/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener
💤 Files with no reviewable changes (1)
  • src/main/resources/META-INF/frontend/styles/static_addon_styles

Comment thread pom.xml
Comment on lines +54 to +75
String handler = """
ev=>{const grid = ev.composedPath()
.find(el => el.matches?.('vaadin-grid-cell-content')).parentElement
grid.preventContextMenu=false;
grid.$contextMenuTargetConnector.openOnHandler.call(grid,ev);
}
""";
var button = new EasyRowAction<T>(null, null, dots, handler);
button.updateRenderer(builder);
column = grid.addColumn(builder.build());
column.setAutoWidth(true);
}

super.update(actions);
}

@Override
protected GridContextMenu<T> createContextMenu() {
var menu = super.createContextMenu();
grid.getElement().executeJs("""
setTimeout(()=>this.$contextMenuTargetConnector.removeListener());
""");

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant files and show compact structure first.
git ls-files | rg '^src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/(DropdownMenuRowActionsRenderer|GridContextMenu|EasyGridWrapper|.*ContextMenu.*|.*RowActions.*)\.java$' || true

printf '\n== outline: DropdownMenuRowActionsRenderer ==\n'
ast-grep outline src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java --view expanded || true

printf '\n== outline: related context menu files ==\n'
for f in $(git ls-files 'src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/*.java' | rg 'ContextMenu|RowActions|EasyGridWrapper|DropdownMenu'); do
  echo "--- $f"
  ast-grep outline "$f" --view expanded || true
done

Repository: FlowingCode/EasyGridAddon

Length of output: 5256


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the target file around the relevant lines, plus related definitions/usages.
file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java'
nl -ba "$file" | sed -n '1,220p'

printf '\n== grep for $contextMenuTargetConnector usage ==\n'
rg -n '\$contextMenuTargetConnector|removeListener\(\)|openOnHandler' src/main/java -S

printf '\n== show related GridContextMenu / context menu classes ==\n'
for f in $(git ls-files 'src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/*.java' | rg 'ContextMenu|DropdownMenu|RowActions|EasyGridWrapper'); do
  echo "--- $f"
  nl -ba "$f" | sed -n '1,260p' | rg -n '\$contextMenuTargetConnector|removeListener\(\)|openOnHandler|context menu|createContextMenu|update\(' -n -C 3 || true
done

Repository: FlowingCode/EasyGridAddon

Length of output: 203


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# If the code references Vaadin private APIs, inspect whether the implementation
# already isolates per-grid state or shares a listener across instances.
rg -n 'class .*GridContextMenu|extends GridContextMenu|createContextMenu\(|removeListener\(|openOnHandler|contextMenuTargetConnector' src/main/java src/test/java -S

Repository: FlowingCode/EasyGridAddon

Length of output: 1332


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java'
echo "== $file =="
sed -n '1,220p' "$file"

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java'
echo
echo "== $file =="
sed -n '1,220p' "$file"

printf '\n== any tests/usages for DropdownMenuRowActionsRenderer ==\n'
rg -n 'DropdownMenuRowActionsRenderer|createContextMenu\(|removeListener\(|openOnHandler|contextMenuTargetConnector' src/test src/main -S

Repository: FlowingCode/EasyGridAddon

Length of output: 9112


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java'
echo "== $file =="
sed -n '1,220p' "$file"

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java'
echo
echo "== $file =="
sed -n '1,220p' "$file"

printf '\n== any tests/usages for DropdownMenuRowActionsRenderer ==\n'
rg -n 'DropdownMenuRowActionsRenderer|createContextMenu\(|removeListener\(|openOnHandler|contextMenuTargetConnector' src/test src/main -S

Repository: FlowingCode/EasyGridAddon

Length of output: 9112


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java'
echo "== $file =="
sed -n '1,220p' "$file"

file='src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java'
echo
echo "== $file =="
sed -n '1,220p' "$file"

printf '\n== any tests/usages for DropdownMenuRowActionsRenderer ==\n'
rg -n 'DropdownMenuRowActionsRenderer|createContextMenu\(|removeListener\(\)|openOnHandler|contextMenuTargetConnector' src/test src/main -S

Repository: FlowingCode/EasyGridAddon

Length of output: 9112


🌐 Web query:

Vaadin Grid addContextMenu removeListener $contextMenuTargetConnector openOnHandler multiple GridContextMenu same grid

💡 Result:

In Vaadin, the GridContextMenu component is designed to be associated with a Grid to provide row-specific context menus [1][2]. Below are the technical insights regarding your specific concerns: Multiple GridContextMenu instances on the same grid: It is possible to attach multiple GridContextMenu instances to a single Grid, though it is often considered a workaround [3]. A common pattern to enable both left and right-click to open a context menu involves using one dummy GridContextMenu and a second functional instance [3]. In this setup, you can programmatically close the dummy menu in a listener to handle the desired interaction flow [3]. Removing Listeners and Managing State: There is no explicit removeContextMenu method on the Grid component [4]. To effectively manage dynamic content or listeners within a GridContextMenu, you should use the setDynamicContentHandler method [5][6]. This allows you to update the menu items immediately before it opens, based on the specific row or column clicked [5][7]. If you are adding listeners to individual menu items dynamically, ensure they are registered and handled within the dynamic content update cycle to avoid issues where click listeners fail to trigger [8]. Handling $contextMenuTargetConnector and openOnHandler: These are internal client-side implementation details of the Vaadin Flow component architecture [9]. Developers generally do not interact with $contextMenuTargetConnector directly. If you encounter issues where the context menu fails to re-attach or function after the Grid is re-added to a layout, a common workaround is to manually re-initialize the connector using JavaScript via the Page object: e.getUI.getPage.executeJavaScript("window.Vaadin.Flow.contextMenuConnector.init($0)", targetElement); [9] For advanced scenarios—such as needing to identify which column was clicked—the standard approach involves adding a listener to the Grid's element to intercept the native contextmenu event, capturing the target column data, and then using that information when the menu is subsequently populated [5][6]. Top results: [5][6][9][3][8]

Citations:


🌐 Web query:

Vaadin GridContextMenu multiple context menus same grid $contextMenuTargetConnector removeListener single connector

💡 Result:

In Vaadin, a Grid component is designed to be associated with one GridContextMenu instance at a time using the standard grid.addContextMenu() API [1][2]. Using multiple GridContextMenu instances attached to the same grid is not a supported use case and can lead to unpredictable behavior, such as conflicting event listeners or UI issues [1]. The contextMenuTargetConnector is an internal Vaadin flow mechanism used to bind the ContextMenu component to its target element (the Grid) [3][1]. It is not intended for direct manual manipulation, and attempting to use removeListener on this connector is not a standard or stable way to swap menus [1][4]. If you need to display different context menu options, the recommended approach is to use a single GridContextMenu instance and update its content dynamically based on the target row or other criteria [5][6][7]: 1. Use setDynamicContentHandler(handler): This method allows you to execute logic immediately before the context menu is opened. You can use it to determine which items should be shown or enabled based on the selected row or column [6]. 2. Use addGridContextMenuOpenedListener(event -> ...): This listener is triggered after the menu is opened. It provides access to the clicked item and column, allowing you to re-populate the menu if necessary [6][7]. If you must switch between entirely different menu structures, you should clear the existing items using menu.removeAll() within the dynamic handler or opened listener and add the required new items programmatically [1][6][7]. Attempting to attach and remove multiple GridContextMenu instances is not supported and there is no public API to "remove" a context menu from a grid once added [4]. Top results: [5], [1], [6], [7]

Citations:


Avoid relying on Vaadin’s private context-menu connector here. grid.$contextMenuTargetConnector.openOnHandler / removeListener() are internal Flow details, so this right-click suppression is version-sensitive and may break on Vaadin upgrades.

🤖 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/easygrid/actions/DropdownMenuRowActionsRenderer.java`
around lines 54 - 75, The DropdownMenuRowActionsRenderer is calling Vaadin’s
private context-menu connector through
grid.$contextMenuTargetConnector.openOnHandler and removeListener(), which is
version-sensitive. Update the handling in createContextMenu() and the row-action
handler setup to use the public Grid/GridContextMenu APIs for opening and
managing the context menu, and remove any direct executeJs access to the
internal connector.

✅ Addressed in commit b2ba085

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the dropdown mode I need a GridContextMenu that only opens programatically. API does not provide a mechanism for disabling the event. I cannot use a popover or context menu because I need to know which item was clicked.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javier-godoy, understood — that's a real Vaadin API gap. GridContextMenu doesn't expose a public way to suppress its default right-click trigger while still receiving the per-row item context, so accessing $contextMenuTargetConnector.openOnHandler / removeListener() is the only viable path here. Withdrawing the comment.


✏️ Learnings added
Learnt from: javier-godoy
Repo: FlowingCode/EasyGridAddon PR: 16
File: src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java:54-75
Timestamp: 2026-06-24T17:00:02.996Z
Learning: In `DropdownMenuRowActionsRenderer.java` (EasyGridAddon, Vaadin Flow), the renderer intentionally uses Vaadin's internal JS connector (`grid.$contextMenuTargetConnector.openOnHandler` / `removeListener()`) to suppress the default right-click trigger on `GridContextMenu` while opening it programmatically from an overflow button. This is a known workaround for a Vaadin public-API gap: there is no supported mechanism to disable the right-click event on `GridContextMenu` while retaining per-row item context. Do not flag these private-connector calls as a bug or suggest switching to a popover/plain context menu — they are the only viable implementation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: javier-godoy
Repo: FlowingCode/EasyGridAddon PR: 10
File: src/test/java/com/flowingcode/vaadin/addons/easygrid/ColumnConfigurationDemo.java:77-79
Timestamp: 2026-05-14T11:59:45.173Z
Learning: When reviewing EasyGridAddon code that registers a formatter via `EasyColumn.setFormatter(...)` (i.e., the lambda parameter `val`), assume `val` is non-null. `ColumnConfigurationTextRenderer` guards `formatter.apply(val)` with a null check and returns `config.getNullRepresentation()` for `val == null`, so formatter lambdas registered through `setFormatter` should not be flagged as “null-unsafe” for dereferencing `val` without an explicit null check. Only flag null-safety issues if the formatter is called from a path that does not follow this guard.

Comment thread src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java Outdated
Comment thread src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java Outdated
Comment thread src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java (1)

96-99: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the debug System.out.println artifact.

The message "WAS STALE... I guess I should fix this?" is a leftover debug print that also signals an unresolved doubt about the stale-element handling. Either drop the print (the return false fallback is the intended behavior and is already documented in the comment above) or replace it with proper logging.

Suggested change
     } catch (StaleElementReferenceException e) {
-      System.out.println("WAS STALE... I guess I should fix this?");
       return false;
     }

If the stale path needs more than a silent fallback, I can help draft a retry/wait around the menu-open probe.

🤖 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/easygrid/it/EasyRowActionIT.java`
around lines 96 - 99, Remove the leftover debug print in EasyRowActionIT’s
stale-element handling and keep the existing fallback behavior. In the helper
that catches StaleElementReferenceException, drop the System.out.println
artifact (or replace it with proper test logging if needed) and leave the return
false path intact, since the surrounding logic already documents that this is
the intended outcome.
♻️ Duplicate comments (1)
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java (1)

69-84: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard the release so a failed/interrupted acquire() cannot over-release the permit.

Moving the release into @After correctly holds the permit for the whole test lifecycle. However, tearDown() now releases unconditionally. JUnit still runs @After if semaphore.acquire() is interrupted (throws) before a permit is obtained, so release() would add a permit that was never acquired, pushing capacity above 4 and weakening the intended browser cap. Track acquisition with a flag and only release when held.

Suggested guard
   `@Before`
   public void setup() throws Exception {
     semaphore.acquire();
+    setupPermitAcquired = true;
     if (isUsingHub()) {
       super.setup();
     } else {
       setDriver(TestBench.createDriver(new ChromeDriver()));
     }
     getDriver().get(getURL(route));
     getCommandExecutor().waitForVaadin();
   }

   `@After`
   public void tearDown() throws Exception {
-    semaphore.release();
+    if (setupPermitAcquired) {
+      setupPermitAcquired = false;
+      semaphore.release();
+    }
   }

Add the field alongside the semaphore:

   private final static Semaphore semaphore = new Semaphore(4);
+  private boolean setupPermitAcquired;
🤖 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/easygrid/it/AbstractViewTest.java`
around lines 69 - 84, The semaphore handling in AbstractViewTest should only
release a permit if setup() actually acquired one, since tearDown() can still
run after an interrupted or failed semaphore.acquire(). Add a simple acquisition
flag alongside the semaphore in AbstractViewTest, set it only after acquire
succeeds, and have tearDown() conditionally call semaphore.release() based on
that flag so the browser cap cannot be over-released.
🧹 Nitpick comments (1)
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java (1)

488-489: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse closeMenu() instead of the raw ESCAPE + wait.

This duplicates the closeMenu() helper (lines 114-117), which already sends ESCAPE and waits for !isContextMenuOpen(). Using the helper keeps the close behavior consistent with the rest of the suite (e.g., the recently updated line 246).

Suggested change
     grid.getCell(0, 0).contextClick();
     waitUntil(d -> isContextMenuOpen());
-    new org.openqa.selenium.interactions.Actions(getDriver()).sendKeys(Keys.ESCAPE).perform();
-    waitUntil(d -> !isContextMenuOpen());
+    closeMenu();
🤖 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/easygrid/it/EasyRowActionIT.java`
around lines 488 - 489, Replace the duplicated ESCAPE key sequence in
EasyRowActionIT with the existing closeMenu() helper so the test uses one
consistent menu-closing path. Locate the direct
Actions(getDriver()).sendKeys(Keys.ESCAPE).perform() and waitUntil(d ->
!isContextMenuOpen()) calls and replace them by invoking closeMenu(), matching
the helper already defined in the class and used elsewhere in the suite.
🤖 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.

Outside diff comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java`:
- Around line 96-99: Remove the leftover debug print in EasyRowActionIT’s
stale-element handling and keep the existing fallback behavior. In the helper
that catches StaleElementReferenceException, drop the System.out.println
artifact (or replace it with proper test logging if needed) and leave the return
false path intact, since the surrounding logic already documents that this is
the intended outcome.

---

Duplicate comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java`:
- Around line 69-84: The semaphore handling in AbstractViewTest should only
release a permit if setup() actually acquired one, since tearDown() can still
run after an interrupted or failed semaphore.acquire(). Add a simple acquisition
flag alongside the semaphore in AbstractViewTest, set it only after acquire
succeeds, and have tearDown() conditionally call semaphore.release() based on
that flag so the browser cap cannot be over-released.

---

Nitpick comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java`:
- Around line 488-489: Replace the duplicated ESCAPE key sequence in
EasyRowActionIT with the existing closeMenu() helper so the test uses one
consistent menu-closing path. Locate the direct
Actions(getDriver()).sendKeys(Keys.ESCAPE).perform() and waitUntil(d ->
!isContextMenuOpen()) calls and replace them by invoking closeMenu(), matching
the helper already defined in the class and used elsewhere in the suite.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0b8abbc2-e3d5-41e1-83b8-4aff8cac683b

📥 Commits

Reviewing files that changed from the base of the PR and between 178826b and 18b461c.

📒 Files selected for processing (5)
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java

@paodb

paodb commented Jun 24, 2026

Copy link
Copy Markdown
Member

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

♻️ Duplicate comments (1)
src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java (1)

67-83: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

The permit is still released before the browser is actually cleaned up.

Because this release happens in the subclass @After, JUnit runs it before inherited teardown logic closes the driver, so the 4-browser cap is lifted too early again. It also releases unconditionally even if Line 71 never finished acquiring a permit.

Suggested fix
-  private final static Semaphore semaphore = new Semaphore(4);
+  private static final Semaphore semaphore = new Semaphore(4);
+  private boolean permitAcquired;

   `@Before`
   public void setup() throws Exception {
     semaphore.acquire();
+    permitAcquired = true;
     if (isUsingHub()) {
       super.setup();
     } else {
       setDriver(TestBench.createDriver(new ChromeDriver()));
     }
     getDriver().get(getURL(route));
     getCommandExecutor().waitForVaadin();
   }

   `@After`
   public void tearDown() throws Exception {
-    semaphore.release();
+    try {
+      super.tearDown();
+    } finally {
+      if (permitAcquired) {
+        permitAcquired = false;
+        semaphore.release();
+      }
+    }
   }
🤖 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/easygrid/it/AbstractViewTest.java`
around lines 67 - 83, The semaphore permit is being released too early in
AbstractViewTest. Move the release so it happens only after the browser cleanup
from the inherited teardown has completed, and make sure the same setup path
that acquires the permit in setup() is the only path that can release it in
tearDown(). Use the AbstractViewTest setup/tearDown methods and the semaphore
field to preserve the 4-browser cap until the driver is fully closed.
🤖 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 `@FEATURE_ROW_ACTIONS.md`:
- Around line 120-130: Update the Row Actions documentation to make it clear
that tooltip and styling APIs are not yet renderer-agnostic. In the sections for
EasyRowAction.tooltip(...), addClassName(...), getStyle(), and
addThemeVariants(...), scope the behavior to INLINE_BUTTONS or add an explicit
caveat that dropdown/context-menu renderers do not currently forward these
settings. Use the EasyRowAction and RowActionsStyle references to keep the
wording aligned across the affected sections.
- Around line 136-140: The withConfirmation API docs in EasyRowAction currently
read like fully generic confirmation support, but the implementation still
hardcodes the dialog buttons as “Ok” and “Cancel”. Update the documentation near
withConfirmation(String message) and withConfirmation(String title, String
message) to explicitly say the confirmation button labels are fixed English for
now and that localization/customization is deferred, so the public API
description matches the current behavior.

In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java`:
- Around line 63-65: The action column in DropdownMenuRowActionsRenderer is
still allowed to expand because only setAutoWidth(true) is applied. Update the
column configuration right after grid.addColumn(builder.build()) to also call
setFlexGrow(0) on the same column so the ⋮ trigger stays compact and does not
stretch with leftover grid space.

In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java`:
- Around line 475-479: The withFunction(SerializableBiConsumer<T, JsonArray>)
method in LitRendererBuilder currently allows handlers to be registered even
when staticOnly() was used, which defers the unsupported-state failure until
later. Add an immediate guard in withFunction() to reject static-only builders
before adding the handler, using the same state tracked by
staticOnly()/requireNotClosed(). Keep the exception at the withFunction() call
site so misuse is caught early and the builder behavior stays consistent with
its documented contract.

In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRowActionsRenderer.java`:
- Line 25: The renderer property key generation in LitRowActionsRenderer is
still collision-prone because ThreadLocalRandom can eventually repeat a previous
value. Replace the random-based key generation with a monotonic counter so each
property name is guaranteed unique across refreshes, and update the affected
property-name creation logic in the renderer setup to use that counter
consistently instead of ThreadLocalRandom.

In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java`:
- Around line 191-207: The confirmation-dialog tests are reusing the same flow
immediately after a cancel without waiting for the dialog to fully close. Update
the relevant assertions in testConfirmation() and testDropdownConfirmation() to
wait for the ConfirmDialogElement to disappear after clicking the cancel button
before reopening it, using the existing dialog lookup/wait pattern in
EasyRowActionIT; leave the final confirm-click paths unchanged since they do not
need the extra close wait.
- Around line 91-94: The isContextMenuOpen helper can still throw during overlay
teardown when a ContextMenuElement becomes stale. Update isContextMenuOpen() in
EasyRowActionIT to catch StaleElementReferenceException around the
stream/element checks and treat that case as closed by returning false, so
closeMenu() can safely continue during detach timing windows.

In `@src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDemo.java`:
- Around line 72-76: The last action in RowActionsDemo is currently icon-only,
which showcases an inaccessible pattern. Update the example to use the row
action API in a way that provides both a text label and an icon, ideally via the
addRowAction call that accepts label and icon providers for the dynamic
subscriber action. Keep the existing star icon behavior, but ensure the rendered
control always has a usable label alongside the icon.

---

Duplicate comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java`:
- Around line 67-83: The semaphore permit is being released too early in
AbstractViewTest. Move the release so it happens only after the browser cleanup
from the inherited teardown has completed, and make sure the same setup path
that acquires the permit in setup() is the only path that can release it in
tearDown(). Use the AbstractViewTest setup/tearDown methods and the semaphore
field to preserve the 4-browser cap until the driver is fully closed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5ad8503-3b62-4bb0-8186-dcd38dbe3640

📥 Commits

Reviewing files that changed from the base of the PR and between 2877880 and b2ba085.

📒 Files selected for processing (31)
  • FEATURE_ROW_ACTIONS.md
  • pom.xml
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/EasyGridWrapper.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/Constant.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/ContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/HasRowActions.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsManager.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsStyle.java
  • src/main/resources/META-INF/frontend/fc-dynamic-buttons.css
  • src/main/resources/META-INF/frontend/fc-icon.ts
  • src/main/resources/META-INF/frontend/styles/static_addon_styles
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/EasyGridDemoView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsMenuDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilderTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITCallables.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/RmiEasyRowAction.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/ViewInitializerImpl.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/test/EasyRowActionUnitTest.java
  • src/test/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener
💤 Files with no reviewable changes (1)
  • src/main/resources/META-INF/frontend/styles/static_addon_styles

Comment thread FEATURE_ROW_ACTIONS.md
Comment thread FEATURE_ROW_ACTIONS.md

import com.vaadin.flow.component.grid.Grid;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the renderer property key collision-free.

ThreadLocalRandom lowers the chance of reuse, but it does not guarantee uniqueness. After enough refreshes this can repeat an older property name and bring back the stale-binding collision this method is meant to prevent. A monotonic counter is safer here.

Suggested fix
-import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicLong;
@@
-  private static final int TAIL = Character.MAX_RADIX * Character.MAX_RADIX * Character.MAX_RADIX;
+  private static final AtomicLong PROPERTY_SEQ = new AtomicLong();
 
   private static String getProperty() {
-    // Random property name ensures no collisions between consecutive renderer updates
-    int n = ThreadLocalRandom.current().nextInt(26 * 10 * TAIL);
-    char letter = (char) ('a' + n / (10 * TAIL));
-    char digit = (char) ('0' + (n / TAIL) % 10);
-    String tail = Integer.toString(TAIL + n % TAIL, Character.MAX_RADIX).substring(1);
-    return String.valueOf(letter) + digit + tail;
+    return "p" + Long.toUnsignedString(PROPERTY_SEQ.incrementAndGet(), Character.MAX_RADIX);
   }

Also applies to: 48-57

🤖 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/easygrid/actions/LitRowActionsRenderer.java`
at line 25, The renderer property key generation in LitRowActionsRenderer is
still collision-prone because ThreadLocalRandom can eventually repeat a previous
value. Replace the random-based key generation with a monotonic counter so each
property name is guaranteed unique across refreshes, and update the affected
property-name creation logic in the renderer setup to use that counter
consistently instead of ThreadLocalRandom.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to human reviewer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javier-godoy, understood — I'll leave this for the human reviewer to weigh in on.


🧠 Learnings used
Learnt from: javier-godoy
Repo: FlowingCode/EasyGridAddon PR: 10
File: src/test/java/com/flowingcode/vaadin/addons/easygrid/ColumnConfigurationDemo.java:77-79
Timestamp: 2026-05-14T11:59:45.173Z
Learning: When reviewing EasyGridAddon code that registers a formatter via `EasyColumn.setFormatter(...)` (i.e., the lambda parameter `val`), assume `val` is non-null. `ColumnConfigurationTextRenderer` guards `formatter.apply(val)` with a null check and returns `config.getNullRepresentation()` for `val == null`, so formatter lambdas registered through `setFormatter` should not be flagged as “null-unsafe” for dereferencing `val` without an explicit null check. Only flag null-safety issues if the formatter is called from a path that does not follow this guard.

@paodb

paodb commented Jun 24, 2026

Copy link
Copy Markdown
Member

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java (1)

359-371: 📐 Maintainability & Code Quality | 🔵 Trivial

Delete the unused BindingType.read(...) methods

copyAttributes(...) already inlines the prefix switch, and nothing else calls the enum’s read(...) overloads. Removing them keeps the dispatch logic in one place.

🤖 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/easygrid/actions/LitRendererBuilder.java`
around lines 359 - 371, Delete the unused BindingType.read(...) overloads since
copyAttributes(...) in LitRendererBuilder now performs the prefix-based read
logic inline and no other code references those methods. Remove the redundant
enum methods and keep the attribute/property dispatch centralized in
copyAttributes(...) using BindingType.of(name), preserving the existing behavior
for ATTRIBUTE versus property bindings.
🤖 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.

Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java`:
- Around line 359-371: Delete the unused BindingType.read(...) overloads since
copyAttributes(...) in LitRendererBuilder now performs the prefix-based read
logic inline and no other code references those methods. Remove the redundant
enum methods and keep the attribute/property dispatch centralized in
copyAttributes(...) using BindingType.of(name), preserving the existing behavior
for ATTRIBUTE versus property bindings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 06f82414-a370-4a48-a0f3-c57682650d45

📥 Commits

Reviewing files that changed from the base of the PR and between 2877880 and 257a3bf.

📒 Files selected for processing (31)
  • FEATURE_ROW_ACTIONS.md
  • pom.xml
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/EasyGridWrapper.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/Constant.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/ContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/HasRowActions.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsManager.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsStyle.java
  • src/main/resources/META-INF/frontend/fc-dynamic-buttons.css
  • src/main/resources/META-INF/frontend/fc-icon.ts
  • src/main/resources/META-INF/frontend/styles/static_addon_styles
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/EasyGridDemoView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsMenuDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilderTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITCallables.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/RmiEasyRowAction.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/ViewInitializerImpl.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/test/EasyRowActionUnitTest.java
  • src/test/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener
💤 Files with no reviewable changes (1)
  • src/main/resources/META-INF/frontend/styles/static_addon_styles

@javier-godoy javier-godoy marked this pull request as ready for review June 24, 2026 19:07
@javier-godoy javier-godoy marked this pull request as draft June 24, 2026 19:30
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@javier-godoy

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/easygrid/actions/LitRowActionsRenderer.java`:
- Around line 66-68: The inline actions column in LitRowActionsRenderer is still
allowed to expand because only setAutoWidth(true) is applied. Update the column
setup in the branch that creates the column with grid.addColumn(litRenderer) so
it also sets flex growth to zero, matching the behavior used in
DropdownMenuRowActionsRenderer for its trigger column. Keep the existing
auto-width sizing, but explicitly disable flex growth on that column so it stays
compact.

In
`@src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java`:
- Around line 67-83: Guard the Semaphore lifecycle in AbstractViewTest so
tearDown() only releases a permit if setup() successfully acquired one. Add a
per-test boolean or similar state around semaphore.acquire() in setup(), set it
only after acquisition succeeds, and check that flag in tearDown() before
calling semaphore.release(). Keep the fix local to AbstractViewTest and its
setup()/tearDown() methods.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9acd02b-187d-41bf-a21c-f84de53c2567

📥 Commits

Reviewing files that changed from the base of the PR and between 2877880 and b9fd620.

📒 Files selected for processing (31)
  • FEATURE_ROW_ACTIONS.md
  • pom.xml
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/EasyGridWrapper.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/AbstractContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/Constant.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/ContextMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/DropdownMenuRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowAction.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/HasRowActions.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilder.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsManager.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsRenderer.java
  • src/main/java/com/flowingcode/vaadin/addons/easygrid/actions/RowActionsStyle.java
  • src/main/resources/META-INF/frontend/fc-dynamic-buttons.css
  • src/main/resources/META-INF/frontend/fc-icon.ts
  • src/main/resources/META-INF/frontend/styles/static_addon_styles
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/EasyGridDemoView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsDynamicDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/RowActionsMenuDemo.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/EasyRowActionTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/actions/LitRendererBuilderTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionIT.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITCallables.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/EasyRowActionITView.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/RmiEasyRowAction.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/it/ViewInitializerImpl.java
  • src/test/java/com/flowingcode/vaadin/addons/easygrid/test/EasyRowActionUnitTest.java
  • src/test/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener
💤 Files with no reviewable changes (1)
  • src/main/resources/META-INF/frontend/styles/static_addon_styles

Comment thread src/test/java/com/flowingcode/vaadin/addons/easygrid/it/AbstractViewTest.java Outdated
@javier-godoy javier-godoy requested review from paodb and scardanzan June 24, 2026 22:51
@javier-godoy javier-godoy marked this pull request as ready for review June 24, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Do

Development

Successfully merging this pull request may close these issues.

2 participants