unmade / am-date-picker

angular material date picker
MIT License
21 stars 13 forks source link

Can't cancel date edit #2

Closed SirDarquan closed 8 years ago

SirDarquan commented 8 years ago

Playing with this more, I noticed that if I open the date picker and select a new date but then either press escape or click outside the dialog area, the date field is updated to the selected date. I would expect the date value not to be updated unless I press the confirmation button.

unmade commented 8 years ago

That's a good question to think. I think it's ok to update the data field to the selected date even if 'OK' (maybe it should be called 'Close') button wasn't pressed. In my own experience many pickers have the same logic and people are expecting the same. In my mind, the 'OK' button is for users, that can't press esc or click outside the dialog area (for example on mobile devices it's not easy to do). Also, maybe I should show 'Clear' button in the picker if amAllowCancel is true. I think I can work on this in nearest weekends.

SirDarquan commented 8 years ago

So what about having an option for auto-save where true works like it is now and changes the 'OK' button to 'Close' and auto-save false means it won't update the date value and 'OK' needs to be used to confirm a change in value.

unmade commented 8 years ago

Sorry it took so long. I've added "Cancel" button. If you press it, the date value won't be updated, otherwise it will. Don't think I will add auto-save option. Click outside to close picker and confirm selected date totally seems more natural to me, and what more important it's faster than every time find 'OK' button and click it.

SirDarquan commented 8 years ago

I don't think you needed the 'Cancel' button. I believe that most people understand clicking outside a dialog and pressing escape closes it out but I guess it doesn't hurt. Just give me a way to hide it if I'd like, maybe by setting the Cancel text to null or something.

unmade commented 8 years ago

I updated master branch. For now, to show 'Cancel' button you should provide the text for it with 'am-cancel-button' attr or set it with amDatePickerConfig.

unmade commented 8 years ago

Could I close the issue?