spec: align FEATURE_ROW_ACTIONS.md with implementation#11
Conversation
Rename withVisibleWhen→visibleWhen, withEnabledWhen→enabledWhen, withTooltip→tooltip in spec and usage examples to match the implementation and Vaadin's component-style naming convention.
The four-parameter overload was never implemented. Remove it from the spec and update the usage example to match the actual API.
Replace the single VaadinIcon overload with the full set implemented: label-only, Icon, IconFactory, icon-only, and per-row ValueProvider variants. Update usage examples accordingly.
Replace SerializableFunction<T,String> with ValueProvider<T,String>, the idiomatic Vaadin type for per-row value providers.
Document the removal API that exists in the implementation but was absent from the spec. Add usage example showing both call sites.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough
ChangesRow Actions API Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| // Removing an action | ||
| EasyRowAction<T> adminAction = easyGrid.addRowAction("Purge", VaadinIcon.TRASH, item -> purge(item)); | ||
| // later: | ||
| easyGrid.removeRowAction(adminAction); |
There was a problem hiding this comment.
removeRowAction and EasyRowAction.remove() are introduced, but the spec doesn't pin down their behavior at the edges. Maybe is worth specifying things like:
- Calling
remove()twice (orremoveRowAction(a)aftera.remove()): no-op orIllegalStateException? - Whether the
EasyRowActionreference is still usable after removal (can it be re-added, or is its state considered dead?) - When the removal becomes visible: immediate re-render or next data refresh?
What do you think?
There was a problem hiding this comment.
Added some clarification on the behaviors
github-project-automation
Bot
moved this from To Do
to In Progress
in Flowing Code Addons
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Line 96: The example uses a generic type parameter T without defining it;
change the declaration to use the concrete type used by the grid (e.g.
EasyRowAction<Person> adminAction) so it matches the grid's type parameter used
elsewhere (easyGrid) and the lambda purge(item) signature; update the
EasyRowAction instantiation to EasyRowAction<Person> and ensure the item
parameter type aligns with the purge(Person) method.
🪄 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: 5c63cfbd-213d-4fb6-80b9-79414a7c8f3f
📒 Files selected for processing (1)
FEATURE_ROW_ACTIONS.md
2 participants
Summary
Drop
withprefix fromEasyRowActionfluent setters (visibleWhen,enabledWhen,tooltip): thewithprefix is a builder-pattern convention for separate builder objects.EasyRowActionis the domain object itself; Vaadin's component-style convention uses bare verb names for self-returning configurators.Remove
addRowAction(..., ButtonVariant, ...)overload: a singleButtonVariantslot cannot express multiple variants and cannot set arbitrary theme strings. The right shape for theme-variant support is a dedicatedaddThemeVariants(ButtonVariant...)method onEasyRowAction, not a factory-method parameter.Expand
addRowActionoverload set: acceptingAbstractIcon<ICON>andValueProvider<T,ICON>is more general thanVaadinIconalone — it covers Lumo icons, custom SVGs, and any icon library. Label-only and icon-only overloads reduce boilerplate for the common cases.VaadinIconimplementsIconFactory, so existing call sites are unaffected.Replace
SerializableFunctionwithValueProviderfor the dynamic tooltip:ValueProvider<T,V>is the idiomatic Vaadin type for "given a bean, produce a value". It is consistent with the per-row icon overloads and signals intent more clearly than the genericSerializableFunction. Both are@FunctionalInterface, so no call-site changes are required.Add
removeRowAction/EasyRowAction.remove(): row-action removal is a basic lifecycle operation needed whenever actions are registered conditionally. Two entry points are provided because callers may hold a reference to the grid, to the action, or to both.Summary by CodeRabbit