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.
|
Warning Review limit reached
More reviews will be available in 17 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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?
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.