"Show Next Dates" fixes

[paodb]

Fixes for #19, #20, #21 & #22

Summary by CodeRabbit

• Style

  • Adjusted button styling and set grid height to a fixed 108px for improved layout.
  • Restored visibility of the grid header for clearer column context.

• Refactor

  • Simplified generation and feeding of next-date entries to the grid, reducing internal complexity and streamlining data flow.

[coderabbitai]

Walkthrough

Refactors next-date generation to use a single CronExpression instance and direct item setting; removes a helper and a theme variant; changes grid height to 108px; and makes the vaadin-grid header visible by removing the CSS rule that hid it.

Changes

Cohort / File(s)	Change Summary
CronExpressionField component logic src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java	Removed calculateNextDate(...); updated generateNextDate(...) to instantiate CronExpression once and iterate via parsedExpression.next(...), returning an empty stream if first next is null; replaced DataProvider.fromCallbacks(...) usage with direct grid.setItems(...) lambda; removed theme variant addition on nextDatesBtn; changed grid height from 30% to 108px.
Grid styling src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css	Removed the rule hiding the vaadin-grid header (vaadin-grid::part(header) { display: none; }), making the grid header visible.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify correctness of the new generateNextDate(...) iteration (single CronExpression and null-first-next handling).
• Confirm the setItems(...) change preserves expected grid behavior (paging/ordering if previously relied on callbacks).
• Check UI impacts from grid height change and visible header.

Possibly related issues

• Scope vaadin-grid header hiding to CronExpressionField only #19 — change directly removes the global header-hiding CSS rule referenced by the issue.

Possibly related PRs

• refactor: component renamed to CronExpressionField #8 — modifies the same CronExpressionField class and related frontend styling; likely related to prior refactors.

Suggested reviewers

• mlopezFC
• javier-godoy

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Show Next Dates' fixes is directly related to the main changes in the pull request, which refactor the next-dates generation logic, adjust UI presentation, and resolve multiple related issues.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch next-dates-fixes

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5872092 and 3f4196a.

📒 Files selected for processing (2)

• src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (2 hunks)
• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css (0 hunks)

💤 Files with no reviewable changes (1)

• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css

🔇 Additional comments (3)

src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (3)

34-34: LGTM!

The Objects import is correctly added to support the Objects::nonNull predicate used in the stream iteration.

---

438-440: LGTM!

The changes simplify the data provisioning by using a lambda-based approach instead of DataProvider, and the fixed grid height aligns with the PR objectives for fixing the "Show Next Dates" functionality.

---

444-452: Excellent fix for the null guard issue!

The implementation correctly addresses the critical issue by:

• Checking if firstNext is null and returning an empty stream
• Using Objects::nonNull as a predicate to stop iteration when no more dates are available
• Starting the stream with the first scheduled date (not the current time)

This properly handles exhausted cron expressions and prevents NPE. The three-argument Stream.iterate(seed, hasNext, next) form is fully supported in the project's target Java versions (17 and 21), so no compatibility concerns exist.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (2)

440-440: Consider the impact of fixed pixel height.

The grid height was changed from a responsive "30%" to a fixed "108px". While this may be intentional to accommodate a specific number of rows plus the now-visible header, it reduces flexibility across different screen sizes and viewport heights.

---

437-441: Add validation before parsing the cron expression.

The method checks if the input is not empty but doesn't validate that it's a valid cron expression before attempting to parse it in generateNextDate(). If an invalid expression somehow reaches this point, CronExpression.parse() will throw an exception.

Apply this diff to add validation:

 private void showNextDates() {
-  if (!inputExpressionTf.getValue().isEmpty()) {
+  if (!inputExpressionTf.getValue().isEmpty() && validateCron(inputExpressionTf.getValue())) {
     grid.removeAllColumns();
     grid.addColumn(date -> formatDate(date));
     grid.setHeight("108px");
     grid.setItems(q -> generateNextDate(LocalDateTime.now(), q.getOffset(), q.getLimit()));
     add(grid);
   }
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6add9d0 and 721cdfb.

📒 Files selected for processing (2)

• src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (1 hunks)
• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css (0 hunks)

💤 Files with no reviewable changes (1)

• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (1)

440-440: Consider using relative height units for better responsiveness.

The hard-coded 108px height may not adapt well to different screen sizes or user preferences. Consider using a relative unit like "10em" or keeping the percentage-based approach.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 721cdfb and 5872092.

📒 Files selected for processing (2)

• src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (1 hunks)
• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css (0 hunks)

💤 Files with no reviewable changes (1)

• src/main/resources/META-INF/frontend/styles/cron-expression-field-styles.css

🔇 Additional comments (1)

src/main/java/com/flowingcode/vaadin/addons/cronexpressionfield/CronExpressionField.java (1)

441-441: LGTM: Cleaner data provider approach.

The direct lambda-based setItems approach is more concise and readable than the previous DataProvider.fromCallbacks pattern.