test: consolidate grid LitRenderer column ITs into one view#9792
test: consolidate grid LitRenderer column ITs into one view#9792web-padawan wants to merge 2 commits intomainvaadin/flow-components:mainfrom chore/consolidate-grid-lit-renderer-itvaadin/flow-components:chore/consolidate-grid-lit-renderer-itCopy head branch name to clipboard
Conversation
Move the LitRenderer-columns showcase (index variable, computed and complex withProperty, @click handlers mutating via refreshItem/refreshAll) from the demo GridViewUsingTemplates page/IT into the dedicated GridLitRendererPage/IT, then delete the demo pair. These exercise LitRenderer features not covered by the existing render/switch/editor tests, so nothing is dropped. Give the existing grid an explicit id so the reattach test no longer depends on DOM order now that the page hosts a second grid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vaadin-review-bot
left a comment
There was a problem hiding this comment.
Reviewed the changes — left 1 comment.
| Finding | |
|---|---|
| 🧹 | Comment describes the old Polymer on-click syntax, but the template below uses Lit @click. |
| .withProperty("address", Person::getAddress)) | ||
| .setHeader("Address"); | ||
|
|
||
| // You can set events handlers associated with the template. The syntax |
There was a problem hiding this comment.
🧹 Comment describes the old Polymer on-click syntax, but the template below uses Lit @click.
The moved comment reads "The syntax follows the Polymer convention "on-event", such as "on-click"", yet the very next line binds the handler with Lit syntax: "<button @click=${handleUpdate}>...". The on-event form belonged to the old TemplateRenderer; LitRenderer uses @event.
The comment was already inaccurate in the deleted demo view, but this PR transplants it into GridLitRendererPage — the canonical LitRenderer example. A developer copying this reference example is told to use on-click, which does not work with LitRenderer. Fix by rewording to describe the @event (e.g. @click) syntax, or drop the comment.
GridLitRendererPage.java:122 · conventions · confirmed
There was a problem hiding this comment.
Removed mention of syntax (comments are demo leftovers, will drop them later).
|
The grid IT module has two LitRenderer test setups: the dedicated
GridLitRendererPage/GridLitRendererIT(routevaadin-grid/lit-renderer) and the older demo-styleGridViewUsingTemplatesPage/GridViewUsingTemplatesIT(routevaadin-grid-it-demo/using-templates, a leftover from the old demo project structure). This keeps only the dedicated pair.The demo pair covered LitRenderer features the dedicated view did not: the
${index}variable, computed and complex-objectwithPropertyvalues, and@clickevent handlers that mutate the item viarefreshItem/refreshAll. That test is moved into the dedicated page/IT before the demo pair is deleted, so no coverage is lost. The migrated grid uses a smaller data set (only the first rows are exercised) to keep the page light.The existing grid now has an explicit id (
lit-renderer-grid) soshouldEnterEditModeAfterReattachselects it by id rather than by DOM position, which is no longer stable now that the page hosts a second grid.Verified:
GridLitRendererITpasses (8 tests, 0 failures). No references to the removed route or element IDs remain.Follow-up to #9787, #9788, #9790, #9791. Part of the grid
vaadin-grid-it-demo/*demo-view consolidation.🤖 Generated with Claude Code