Skip to content

spec: align FEATURE_ROW_ACTIONS.md with implementation#11

Open
javier-godoy wants to merge 5 commits into
spec/row-actionsfrom
spec/row-actions-update-1
Open

spec: align FEATURE_ROW_ACTIONS.md with implementation#11
javier-godoy wants to merge 5 commits into
spec/row-actionsfrom
spec/row-actions-update-1

Conversation

@javier-godoy
Copy link
Copy Markdown
Member

Summary

  • Drop with prefix from EasyRowAction fluent setters (visibleWhen, enabledWhen, tooltip): the with prefix is a builder-pattern convention for separate builder objects. EasyRowAction is the domain object itself; Vaadin's component-style convention uses bare verb names for self-returning configurators.

  • Remove addRowAction(..., ButtonVariant, ...) overload: a single ButtonVariant slot cannot express multiple variants and cannot set arbitrary theme strings. The right shape for theme-variant support is a dedicated addThemeVariants(ButtonVariant...) method on EasyRowAction, not a factory-method parameter.

  • Expand addRowAction overload set: accepting AbstractIcon<ICON> and ValueProvider<T,ICON> is more general than VaadinIcon alone — it covers Lumo icons, custom SVGs, and any icon library. Label-only and icon-only overloads reduce boilerplate for the common cases. VaadinIcon implements IconFactory, so existing call sites are unaffected.

  • Replace SerializableFunction with ValueProvider for 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 generic SerializableFunction. 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.

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.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Warning

Review limit reached

@paodb, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 28867f07-ff8e-404f-ad2d-91ffb46faa44

📥 Commits

Reviewing files that changed from the base of the PR and between de3c379 and 94c028c.

📒 Files selected for processing (1)
  • FEATURE_ROW_ACTIONS.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spec/row-actions-update-1

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 and usage tips.

@javier-godoy javier-godoy marked this pull request as draft May 26, 2026 19:46
@javier-godoy javier-godoy marked this pull request as ready for review May 26, 2026 19:46
@paodb
Copy link
Copy Markdown
Member

paodb commented May 26, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread FEATURE_ROW_ACTIONS.md
// Removing an action
EasyRowAction<T> adminAction = easyGrid.addRowAction("Purge", VaadinIcon.TRASH, item -> purge(item));
// later:
easyGrid.removeRowAction(adminAction);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 (or removeRowAction(a) after a.remove()): no-op or IllegalStateException?
  • Whether the EasyRowAction reference 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?

@github-project-automation github-project-automation Bot moved this from To Do to In Progress in Flowing Code Addons May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants