Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Jan 7, 2026

Close #164

Summary by CodeRabbit

  • Bug Fixes
    • Fixed grid height in empty state so the grid no longer retains unintended space (minimum height is now cleared when empty).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

Walkthrough

When the EMPTY parameter is present, the view now sets the grid element's minimum-height CSS style to "0" in the setParameter method, adding explicit height control for empty-grid scenarios without changing signatures or public API.

Changes

Cohort / File(s) Change Summary
Grid Height Test Configuration
src/test/java/.../HeightByRowsITView.java
Added an else branch in setParameter to set the grid element's min-height CSS style (CSS variable --_grid-min-height) to "0" when the EMPTY parameter is present; adjusts empty-grid height handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • mlopezFC
  • paodb
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 accurately describes the main change: fixing grid-min-height styling in an integration test file.
Linked Issues check ✅ Passed The PR adds min-height CSS styling to fix the grid height issue reported in #164, addressing the failing integration test expectations.
Out of Scope Changes check ✅ Passed The change is narrowly scoped: only modifies HeightByRowsITView to add min-height styling on the EMPTY parameter path, directly addressing issue #164.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfe7fc7 and 29b0416.

📒 Files selected for processing (1)
  • src/test/java/com/flowingcode/vaadin/addons/gridhelpers/it/HeightByRowsITView.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/flowingcode/vaadin/addons/gridhelpers/it/HeightByRowsITView.java

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
@src/test/java/com/flowingcode/vaadin/addons/gridhelpers/it/HeightByRowsITView.java:
- Around line 75-77: The test uses the undocumented internal CSS variable
"--_grid-min-height" via grid.getElement().getStyle().set(...); replace that
with a documented Vaadin approach: use grid.setAllRowsVisible(true) when you
want the grid to size to its content for the non-scrolling case, or set a
documented Lumo sizing token (e.g., set the element style max-height using
"var(--lumo-size-m)" with calc() or a fixed max-height) for controlled sizing;
update the empty-grid branch of the test to assert the behavior of the chosen
documented approach (e.g., call grid.setAllRowsVisible(true) and verify height
for empty grid or set max-height using calc(0 * var(--lumo-size-m)) and assert
the CSS max-height), and remove any usage of "--_grid-min-height".
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 961dbb1 and dfe7fc7.

📒 Files selected for processing (1)
  • src/test/java/com/flowingcode/vaadin/addons/gridhelpers/it/HeightByRowsITView.java

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

@paodb paodb merged commit 4076d75 into master Jan 8, 2026
5 checks passed
@paodb paodb deleted the fix-164 branch January 8, 2026 17:32
@github-project-automation github-project-automation bot moved this from To Do to Pending release in Flowing Code Addons Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending release

Development

Successfully merging this pull request may close these issues.

Vaadin 24: HeightByRowsIT integration tests fail (testOpenEmpty)

3 participants