[#9] Added a date picker calendar widget.#32
Open
AlexSkrypnyk wants to merge 7 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a ChangesDate picker feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WidgetFactory
participant DateWidget
participant DateBounds
User->>WidgetFactory: create date field widget
WidgetFactory->>DateWidget: seed value and bounds
User->>DateWidget: navigate calendar
DateWidget->>DateBounds: clamp cursor
User->>DateWidget: confirm selection
DateWidget-->>User: return ISO date
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 99.51% 99.54% +0.03%
==========================================
Files 70 73 +3
Lines 2082 2221 +139
==========================================
+ Hits 2072 2211 +139
Misses 10 10 ☔ View full report in Codecov by Harness. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Engine/Engine.php (1)
190-204: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject malformed
FieldType::Dateinput inEngine::validateValue()
Tui::collect()feeds raw--prompts/env values intoEngine::collect(), and this path only checksdateBounds;DateBounds::violation()returnsNULLon parse failure, so bad date strings can still be accepted unchanged. Add a strict date parse/type check here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Engine/Engine.php` around lines 190 - 204, The validateValue() method must reject malformed FieldType::Date inputs before applying date bounds. Add a strict type check and date parse validation for fields whose type is FieldType::Date, returning an appropriate validation error when the value is not a valid date; preserve the existing bounds and custom validator behavior for other field types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Schema/SchemaGenerator.php`:
- Around line 53-55: In the schema-generation method, serialize dateBounds
weekStart consistently with other enums by replacing the weekStart ->name access
with ->value, unless the schema explicitly requires the enum case name rather
than its backing value.
---
Outside diff comments:
In `@src/Engine/Engine.php`:
- Around line 190-204: The validateValue() method must reject malformed
FieldType::Date inputs before applying date bounds. Add a strict type check and
date parse validation for fields whose type is FieldType::Date, returning an
appropriate validation error when the value is not a valid date; preserve the
existing bounds and custom validator behavior for other field types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e73611b6-6658-414e-8cc7-42beb57aeb45
⛔ Files ignored due to path filters (8)
docs/assets/widget-date-ascii-no-ansi.svgis excluded by!**/*.svgdocs/assets/widget-date-ascii.svgis excluded by!**/*.svgdocs/assets/widget-date-no-ansi.svgis excluded by!**/*.svgdocs/assets/widget-date.svgis excluded by!**/*.svgdocs/assets/widgets-ascii-no-ansi.svgis excluded by!**/*.svgdocs/assets/widgets-ascii.svgis excluded by!**/*.svgdocs/assets/widgets-no-ansi.svgis excluded by!**/*.svgdocs/assets/widgets.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
README.mddocs/util/update-assets.phpplayground/3-widgets/show.phpplayground/3-widgets/widget-date.phpplayground/3-widgets/widgets.phpsrc/Builder/FieldBuilder.phpsrc/Builder/PanelBuilder.phpsrc/Config/DateBounds.phpsrc/Config/Field.phpsrc/Config/FieldType.phpsrc/Config/Weekday.phpsrc/Engine/Engine.phpsrc/Schema/AgentHelp.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Widget/DateWidget.phpsrc/Widget/WidgetFactory.phptests/phpunit/Unit/Builder/FormTest.phptests/phpunit/Unit/Config/DateBoundsTest.phptests/phpunit/Unit/Config/WeekdayTest.phptests/phpunit/Unit/Engine/EngineDeclaredBehaviourTest.phptests/phpunit/Unit/Resolver/InputResolverTest.phptests/phpunit/Unit/Schema/AgentHelpTest.phptests/phpunit/Unit/Schema/SchemaGeneratorTest.phptests/phpunit/Unit/Schema/SchemaValidatorTest.phptests/phpunit/Unit/Widget/DateWidgetTest.phptests/phpunit/Unit/Widget/WidgetFactoryTest.php
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/9-date-picker
branch
from
778d723 to
e47aff7
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Schema/SchemaValidator.php (1)
116-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame bounds-precedence duplication as
Engine::validateValue().
checkBounds()andcheckDateBounds()are structurally identical aside from which bound property they read, and together they replicate the same "prefer number bounds, fall back to date bounds" precedence duplicated insrc/Engine/Engine.php(Line 196). Consolidating via a sharedField::boundsViolation()helper (mirroring the existingoptionError()precedent) would collapse both methods into one and keep the two validators from silently diverging.♻️ Proposed consolidation
- $bounds_error = $this->checkBounds($field, $value); - if ($bounds_error !== NULL) { - return $bounds_error; - } - - $date_error = $this->checkDateBounds($field, $value); - if ($date_error !== NULL) { - return $date_error; - } + $violation = $field->boundsViolation($value); + if ($violation !== NULL) { + return sprintf('Question "%s" must be %s.', $field->id, $violation); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Schema/SchemaValidator.php` around lines 116 - 138, Consolidate bound selection into a shared Field::boundsViolation() helper, applying the existing precedence of numeric bounds before date bounds. Update SchemaValidator::checkBounds() and checkDateBounds() to use this helper, remove the duplicate date-specific method, and update Engine::validateValue() to use the same centralized logic so all validators remain consistent.src/Engine/Engine.php (1)
195-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the bounds-precedence logic into
Field.This line duplicates the exact same "prefer number bounds, fall back to date bounds" precedence that
SchemaValidator::checkBounds()/checkDateBounds()implement separately (see src/Schema/SchemaValidator.php Lines 116-138).Fieldalready exposesoptionError()as a shared helper consumed by bothEngineandSchemaValidatorfor the analogous options-validation concern — the same pattern here would prevent the two call sites from silently diverging if the precedence or bounds types ever change.♻️ Proposed consolidation (requires adding a method to Field.php, not shown in this batch)
// In src/Config/Field.php public function boundsViolation(mixed $value): ?string { return $this->bounds?->violation($value) ?? $this->dateBounds?->violation($value); }protected function validateValue(Field $field, mixed $value): ?string { - $violation = $field->bounds?->violation($value) ?? $field->dateBounds?->violation($value); + $violation = $field->boundsViolation($value); if ($violation !== NULL) { return sprintf('must be %s.', $violation); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Engine/Engine.php` around lines 195 - 209, Consolidate bounds precedence in Field by adding a boundsViolation(mixed $value): ?string helper that prefers bounds over dateBounds. Update Engine::validateValue and SchemaValidator::checkBounds/checkDateBounds to call this shared method instead of duplicating null-coalescing logic, preserving the existing violation handling and messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 150-152: Update the README configuration example to import
DrevOps\Tui\Config\Weekday alongside the other imports, so the Weekday::Sunday
reference resolves when copied.
In `@src/Builder/FieldBuilder.php`:
- Around line 692-693: The parseBoundDate method currently accepts an
unrestricted string for the min/max selector; introduce a pure or backed enum
representing these two values, update parseBoundDate and related logic to accept
the enum, and pass the enum cases from the min/max calls around the affected
validation block. Use the enum’s value only when constructing exception
messages, including all branches covering the selector.
In `@src/Config/DateBounds.php`:
- Around line 30-35: Validate the bound ordering in the DateBounds constructor:
when both min and max are provided, reject cases where min is later than max,
using the class’s existing exception or validation convention. Keep null bounds
valid and ensure every direct DateBounds instantiation preserves the invariant
consumed by contains(), describe(), and clamp().
In `@src/Widget/WidgetFactory.php`:
- Line 60: The DateWidget currently hard-codes navigation keys instead of using
WidgetFactory’s injected keymap, so custom date bindings are ignored. Update
DateWidget::handle() and move() to resolve navigation through the configured
Action bindings, and ensure WidgetFactory passes that keymap when creating the
DateWidget; add a test covering remapped date navigation.
In `@tests/phpunit/Unit/Widget/DateWidgetTest.php`:
- Around line 201-212: Add symmetric max-bound coverage to
testDimsOutOfRangeDays by creating DateBounds with a max date and asserting a
day after that maximum is rendered with the theme description/dimmed styling,
while preserving the existing cursor-day assertion.
---
Outside diff comments:
In `@src/Engine/Engine.php`:
- Around line 195-209: Consolidate bounds precedence in Field by adding a
boundsViolation(mixed $value): ?string helper that prefers bounds over
dateBounds. Update Engine::validateValue and
SchemaValidator::checkBounds/checkDateBounds to call this shared method instead
of duplicating null-coalescing logic, preserving the existing violation handling
and messages.
In `@src/Schema/SchemaValidator.php`:
- Around line 116-138: Consolidate bound selection into a shared
Field::boundsViolation() helper, applying the existing precedence of numeric
bounds before date bounds. Update SchemaValidator::checkBounds() and
checkDateBounds() to use this helper, remove the duplicate date-specific method,
and update Engine::validateValue() to use the same centralized logic so all
validators remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3f35571a-5b1e-49b2-85df-a56813e95d9e
⛔ Files ignored due to path filters (8)
docs/assets/widget-date-ascii-no-ansi.svgis excluded by!**/*.svgdocs/assets/widget-date-ascii.svgis excluded by!**/*.svgdocs/assets/widget-date-no-ansi.svgis excluded by!**/*.svgdocs/assets/widget-date.svgis excluded by!**/*.svgdocs/assets/widgets-ascii-no-ansi.svgis excluded by!**/*.svgdocs/assets/widgets-ascii.svgis excluded by!**/*.svgdocs/assets/widgets-no-ansi.svgis excluded by!**/*.svgdocs/assets/widgets.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
README.mddocs/util/update-assets.phpplayground/3-widgets/show.phpplayground/3-widgets/widget-date.phpplayground/3-widgets/widgets.phpsrc/Builder/FieldBuilder.phpsrc/Builder/PanelBuilder.phpsrc/Config/DateBounds.phpsrc/Config/Field.phpsrc/Config/FieldType.phpsrc/Config/Weekday.phpsrc/Engine/Engine.phpsrc/Schema/AgentHelp.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Widget/DateWidget.phpsrc/Widget/WidgetFactory.phptests/phpunit/Unit/Builder/FormTest.phptests/phpunit/Unit/Config/DateBoundsTest.phptests/phpunit/Unit/Config/WeekdayTest.phptests/phpunit/Unit/Engine/EngineDeclaredBehaviourTest.phptests/phpunit/Unit/Resolver/InputResolverTest.phptests/phpunit/Unit/Schema/AgentHelpTest.phptests/phpunit/Unit/Schema/SchemaGeneratorTest.phptests/phpunit/Unit/Schema/SchemaValidatorTest.phptests/phpunit/Unit/Widget/DateWidgetTest.phptests/phpunit/Unit/Widget/WidgetFactoryTest.php
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9
Summary
Adds a
datefield type: a month-calendar widget navigable by day, week and month, returning a normalized ISOYYYY-MM-DDstring. The widget is wired through the full stack - builder (minDate/maxDate/weekStart), config (DateBounds,Weekday), factory, engine validation, and JSON schema/agent-help - so the range and week-start declared on a field behave identically whether the value comes from the interactive TUI or headless collection. Navigation is routed through the configurable key map, like every other widget.Changes
src/Widget/DateWidget.phprenders a "Month YYYY" heading, a weekday header ordered from the configured week-start day, and a day grid. Day and week movement and Accept resolve through the injected key map - so the arrow keys, the vim preset'sh/j/k/land any consumer remap all reach them - while PageUp/PageDown change month and Home/End jump to the first/last day of the month. The cursor is always clamped into the declared range; out-of-range days stay visible but dimmed. The widget draws its own key-hint line (rendersHint()returnsTRUE), andplayground/3-widgets/widgets.phpskips the generic hint line for such widgets so the two can't contradict.src/Config/DateBounds.phpholds an optional inclusivemin/max(DateTimeImmutable) and aWeekday, and rejects a minimum declared after the maximum in its own constructor. It provides strictY-m-dparsing (round-tripped so a malformed or rolled-over value like2026-02-30is rejected rather than silently normalized),contains(),clamp(), a humandescribe()range phrase, and aviolation()check shared by the widget, engine and schema validator.src/Config/Weekday.phpis a new ISO-8601-backed enum (Monday = 1...Sunday = 7) withabbreviation(),fromDate(),sequence()(the seven weekdays in display order from a given start) andcolumnOf()(grid column placement).VimKeyMapbindsh/j/k/lto the date scope's move actions - safe because the calendar takes no typed input, like the single-choice list - so the vim preset drives the calendar the same way it drives the panel navigator.PanelBuilder::date()adds a date field;FieldBuilder::minDate(),maxDate()andweekStart()declare the range and first weekday, assembled into aDateBoundsonbuild()viaFieldBuilder::buildDateBounds(). An invalid bound string or a min declared after max throwsConfigException; the date setters are inert when called on a non-date field.Fieldgains adateBoundsproperty alongside the existingbounds;FieldType::Dateis a new enum case;WidgetFactoryseeds aDateWidgetfrom the field's current string value and declared bounds.Engine::validateValue()checksdateBounds?->violation()alongside the existing number-bounds check, so a--promptsor environment value outside the declared range is rejected with the same range phrase the interactive widget shows.SchemaValidatortype-checks date values (an empty string, or a strictY-m-ddate) and enforces the declared range;SchemaGeneratoremitsmin_date,max_dateandweek_starton the field's schema entry;AgentHelpappends a range annotation (orYYYY-MM-DDwhen unbounded) to a date field's help line.playground/3-widgets/widget-date.phpis a new standalone demo;widgets.phpandshow.phpinclude the date widget in the combined showcase;docs/util/update-assets.phpadds an expect script and row count for the date widget and regenerates the four combinedwidgets*.svgassets plus four new per-widgetwidget-date*.svgassets;README.mddocuments thedatefield (usage, bounds, key hints, screenshots) and updates the widget count to include it.DateBoundsTest,WeekdayTestandDateWidgetTestcover the added classes directly - navigation (including vim-preset remapping through an injected key map), bounds clamping and dimming on both edges, week-start layout, the constructor invariant, and boundary/impossible-date cases;FormTest,WidgetFactoryTest,EngineDeclaredBehaviourTest,InputResolverTest,SchemaGeneratorTest,SchemaValidatorTestandAgentHelpTestare extended to cover thedatefield type end to end. The full suite passes with 100% coverage on all new and touched classes.Before / After