Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DatePicker and TimePicker controls #4108

Merged
merged 40 commits into from
Jun 30, 2020
Merged

Conversation

amwx
Copy link
Contributor

@amwx amwx commented Jun 12, 2020

What does the pull request do?

Adds the WinUI style DatePicker and TimePickers & related classes/controls

These are my own implementation, but the APIs are very similar. In WinUI, DatePicker has a both a SelectedDate and Date property, and TimePicker has SelectedDate and Date properties, the selected variants being nullable. I've only included the SelectedDate / SelectedTime properties, as I didn't really see a need for both. Also formatting for DatePicker uses standard .net DateTime formatting rules instead of the UWP/WinUI formatter.

image

image

image

@maxkatz6
Copy link
Member

Should there also be DatePicker style for default theme?
Not sure, how it should be tho. It could be either same style or simplifier to match other default styles.

@amwx amwx changed the title [WIP] Add DatePicker and TimePicker controls Add DatePicker and TimePicker controls Jun 14, 2020
@amwx amwx marked this pull request as ready for review June 14, 2020 23:34
Copy link
Member

@jmacato jmacato left a comment

Choose a reason for hiding this comment

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

looking great! though i just want to point out things

src/Avalonia.Controls/DateTimePickers/DateTimeFormatter.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/DateTimePickers/LoopingPanel.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/DateTimePickers/LoopingSelector.cs Outdated Show resolved Hide resolved
@amwx
Copy link
Contributor Author

amwx commented Jun 16, 2020

Also waiting on #3970 to possibly better handle the popup (popup near screen bounds can cause it to be shown well above where the date/time picker is)

@jmacato
Copy link
Member

jmacato commented Jun 16, 2020

@amwx i merged that PR

@jmacato
Copy link
Member

jmacato commented Jun 17, 2020

@amwx Tis looking great! Though i hope you can remove the custom DateTimeFormatting stuff.. We dont want to maintain such stuff, at least in-repo... and that code reminds me of a certain british guy ranting about time zones in youtube 😆

@amwx
Copy link
Contributor Author

amwx commented Jun 17, 2020

@jmacato It was removed and now uses normal .net formatting

@jmacato
Copy link
Member

jmacato commented Jun 17, 2020

@amwx Thank you very much! Will review this later in detail :)

Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

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

Thanks @amwx - first of all this control looks and works great! However I have some concerns about the implementation - in particular the size. These controls appear to be larger than any other control in the Avalonia codebase which feels excessive given the fact they're date/time pickers.

In addition, LoopingSelector creates 243 DatePickerPresenterItems when the popup is opened!

I will have more feedback, but thought I'd leave my feedback so far now.

src/Avalonia.Controls/DateTimePickers/DatePicker.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/DateTimePickers/DatePicker.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/DateTimePickers/LoopingSelector.cs Outdated Show resolved Hide resolved
@amwx
Copy link
Contributor Author

amwx commented Jun 23, 2020

I've updated all of the controls here to make a few changes that reduce the overall size of this PR.
Some stuff previously managed in code has been moved to Styles to better match avalonia (i.e. popup and date/time pickerpresenter is now exposed in the template). The templates have gotten slightly larger as a result, but not much, but more customization should be available now too.
Controls have been slightly rewritten as a result.
LoopingSelector has been replaced by DateTimePickerPanel which is a simpler, more efficient implementation (ex. 200+ items aren't present at a time)
Code organization has also been fixed

@jmacato
Copy link
Member

jmacato commented Jun 24, 2020

Looking great :D Just tested and it's so fancy :)

Copy link
Member

@jmacato jmacato left a comment

Choose a reason for hiding this comment

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

LGTM for the most part :) Also tested :)

Copy link
Member

@danwalmsley danwalmsley left a comment

Choose a reason for hiding this comment

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

LGTM

@danwalmsley danwalmsley merged commit f73f817 into AvaloniaUI:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants