Date-range-picker Design to use Prev and Next Buttons in History and Logbook on mobiles #23228
Conversation
|
Hi @boern99, Some things I notice:
|
|
@wendevlin: yes thought about 2 PRs but if next-buttons are hidden on mobile second PR would depend on the first PR. |
wendevlin
added
the
Noteworthy
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| .value=${this.timePicker | ||
| ? formatDateTime( | ||
| ? html`<ha-textarea | ||
| mobile2multiline |
There was a problem hiding this comment.
I don't feel like this should be part of ha-textarea. Also use mobile-multiline as a name otherwise.
There was a problem hiding this comment.
needed a way to display it in one-line, my solution was to add "white-space: nowrap;" for desktops: this ignores the nextline \n in the datestring. Have not found a better way change css for ha-textarea from outside
| .label=${this.hass.localize( | ||
| "ui.components.date-range-picker.end_date" | ||
| "ui.components.selectors.selector.types.datetime" |
There was a problem hiding this comment.
Don't use the translation key of another selector, you can reference its value though
There was a problem hiding this comment.
Or use the already existing start_date and end_date? "Start date - End date" would be more explaining as "Date Time" for the range
| Math.max( | ||
| 3600000, | ||
| differenceInMilliseconds(this.endDate, this.startDate) | ||
| let dateRange: [Date, Date]; |
There was a problem hiding this comment.
Lets extract the next and prev functions from hui-energy-period-selector and make them helper functions we can use here too.
There was a problem hiding this comment.
The new functions can deal with any-ranges and also shorter than one day. If selected a week the range is 7 days: so Week is not needed. Extra logic is only needed for month, as the timespans can be different. And the month-Logic can also shift between years without problem.
There was a problem hiding this comment.
Still they share the same logic right? So we can combine them into 1 function we use in both places?
There was a problem hiding this comment.
yes, took now a detailed look at it: you are right: we can combine them. Will upload a draft: hopefully that's the way it should be done ..
| margin-inline-end: 0; | ||
| margin-inline-start: initial; |
There was a problem hiding this comment.
If you remove margin-right, you should also remove these 2
Co-authored-by: Wendelin <[email protected]>
Co-authored-by: Wendelin <[email protected]>
Co-authored-by: Wendelin <[email protected]>
…er-for-History-and-Logbook
|
Changed shiftDateRange-function to be able to shift between timespans lower than one day. |
| export const shiftDateRange = ( | ||
| startDate: Date, | ||
| endDate: Date, | ||
| forward: boolean, | ||
| locale: FrontendLocaleData, | ||
| config: any | ||
| ): { start: Date; end: Date } => { | ||
| let start: Date; | ||
| let end: Date; | ||
| if ( | ||
| (calcDateProperty( | ||
| startDate, | ||
| isFirstDayOfMonth, | ||
| locale, | ||
| config | ||
| ) as boolean) && | ||
| (calcDateProperty(endDate, isLastDayOfMonth, locale, config) as boolean) | ||
| ) { | ||
| const difference = | ||
| ((calcDateDifferenceProperty( | ||
| endDate, | ||
| startDate, | ||
| differenceInMonths, | ||
| locale, | ||
| config | ||
| ) as number) + | ||
| 1) * | ||
| (forward ? 1 : -1); | ||
| start = calcDate(startDate, addMonths, locale, config, difference); | ||
| end = calcDate( | ||
| calcDate(endDate, addMonths, locale, config, difference), | ||
| endOfMonth, | ||
| locale, | ||
| config | ||
| ); | ||
| } else { | ||
| const difference = | ||
| ((calcDateDifferenceProperty( | ||
| endDate, | ||
| startDate, | ||
| differenceInMilliseconds, | ||
| locale, | ||
| config | ||
| ) as number) + | ||
| 1) * | ||
| (forward ? 1 : -1); | ||
| start = calcDate(startDate, addMilliseconds, locale, config, difference); | ||
| end = calcDate(endDate, addMilliseconds, locale, config, difference); | ||
| } | ||
| return { start, end }; | ||
| }; |
There was a problem hiding this comment.
Do you think you can add unit tests for this in a seperate PR?
Breaking change
Proposed change
@marcinbauer85: This is the implementation for the prepared design for #22947. As i discarded the first draft for moving date-range-picker to Top, the original Pull-Request was closed.
This is what it looks like now:
Desktop View:
Mobile View:
if it is this year it hides the year:
@wendevlin: Thank you, yes ha-textarea did the trick. Since you offered, that you can try to implement the design: feel free to make changes if would have done it in a better way ...
The Commit also includes an updated logic to be able to recognice a whole month, so that if for example January is selected and you click on next, it uses whole next month, even the month has a different timespan:
This change allows to remove "Previous month" and "Previous year", as the Previous-Button does exact the same.
I removed Previous Week, Previous Month and Previous Year, so that the date-Selector now better fits also on mobiles:
in comparison this is before the removal, Cancel and Select is depending on the resolution maybe out of view:
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: