feat(ui5-daterange-picker): two months mode is implemented#13196
feat(ui5-daterange-picker): two months mode is implemented#13196
Conversation
|
🚀 Deployed on https://pr-13196--ui5-webcomponents-preview.netlify.app |
…ader focus behaviour
…ther opened picker
GDamyanov
changed the title
| <> | ||
| {/* When pickers are active, show standard calendar header */} | ||
| {this._shouldShowOnePickerHeaderButtonInMultipleMonths && ( | ||
| <div class="ui5-calheader ui5-calheader-multiple ui5-calheader-default-multiple test" exportparts="calendar-header-arrow-button, calendar-header-middle-button"> |
| */ | ||
| _submitClick() { |
There was a problem hiding this comment.
The OK/Cancel footer is rendered on phone, but onSelectedDatesChange (inherited from DatePicker) auto-closes the picker when 2 dates are selected via _togglePicker(). So:
- User selects first date → picker stays open
- User selects second date → picker auto-closes immediately
- OK/Cancel buttons are never reachable
This makes _submitClick, _cancelClick, _submitDisabled, _okButtonText, _cancelButtonText, and the two new i18n keys all effectively dead code.
Fix: Modify onSelectedDatesChange to NOT auto-close when showTwoMonths && isPhone()
| return `${DateRangePicker.i18nBundle.getText(DATETIME_COMPONENTS_PLACEHOLDER_PREFIX)} ${this._lastDateRangeForTheCurrentYear}`; | ||
| } | ||
|
|
||
| get _submitDisabled() { |
There was a problem hiding this comment.
This allows OK button to be enabled with only 1 date, as this component is supposed to be a range pick, I think it would be reasonable to do:
return !this._calendarSelectedDates || this._calendarSelectedDates.length < 2;| return; | ||
| } | ||
|
|
||
| const newValue = this._buildValue(this._startDateTimestamp, this._endDateTimestamp); |
There was a problem hiding this comment.
this line reads from this.value, not from the calendar's current selection
we could extract timestamps from the calendar's _calendarSelectedDates
| @property({ type: Boolean }) | ||
| stretch = false; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Missed JSDoc, please remove it
| font-weight: bold; | ||
| } | ||
|
|
||
| .ui5-daterange-calendar-mobile-footer { |
There was a problem hiding this comment.
this class is not used anywhere
| pointer-events: all; | ||
| background: var(--sapGroup_ContentBackground); | ||
| border-radius: var(--_ui5_popup_border_radius); | ||
| min-height: 2rem; |
There was a problem hiding this comment.
dont we need a variable here and at line 135 to handle compact/cozy scenacrios ?
| } | ||
|
|
||
| .ui5-cal-month-header-container { | ||
| width: 50%; |
There was a problem hiding this comment.
this only assumes exactly two months, can we use flex: 1 for flexibility ?
| }); No newline at end of file | ||
| }); | ||
|
|
||
| describe("DateRangePicker - Two Calendars Feature", () => { |
There was a problem hiding this comment.
We have few missing test cases, for example:
- We have implemented a lot of phone features, we should test some cases. You can use
cy.ui5SimulateDevice("phone"). - Mobile footer OK/Cancel buttons
- Min/Max date constraints with two months
- Dynamic toggle of showTwoMonths after initial render
| @@ -1,5 +1,6 @@ | |||
| .ui5-date-picker-popover { | |||
There was a problem hiding this comment.
With most of the secondary-calendar types, there is slight misalignment.
3 participants
This PR adds a new showTwoCalendars property to the DateRangePicker component that displays two consecutive months side by side, making it easier to select date ranges that span multiple months.
New Property - showTwoMonths (boolean, default: false)