Calendar rebase (4) #1371

fnagel · 2014-10-22T19:14:59Z

This replaces #1352.

Change status caching, fix existing value related methods, introduce $.date construction with date object, selected property is null by default, add selected getter

fnagel · 2014-11-07T17:14:26Z

Changed:

Fixed datepicker destroy test (thanks @jzaefferer)
Removed option getter and valueAsDate method in Calendar Widget
Moved and fixed unit tests accordingly

fnagel · 2014-11-07T17:45:57Z

Fixed lint issues.

jzaefferer · 2014-11-20T13:49:33Z

Doing some testing of demos and visual tests.

Needs a rebase on top of master, since the theme visual test page is gone and we have new font sizes
tests/visual/compound/datepicker_dialog.html, tests/visual/compound/dialog_widgets.html and tests/visual/dialog/complex-dialogs.html use datepicker, needs an update, currently broken
On demos/datepicker/default.html:
- Compared to master, the default date format is different, is that intended? Master has 4 digit years, this branch only 2
- On master, I can't enter various characters, looks like only digits and slash are allowed; on this branch, I can type anything, see the character appear for a split second, then removed. After picking a date, anything in addition is removed. I found this on the wiki, so I think this whole thing needs to be taken out: "constrainInput: Suppressing user input is a UX bad practice. I don't believe we should provide this functionality. - remove"
- In addition to the suppression of the previous point, I also can't use the cursor keys to move the cursor anywhere except for the end of the input.
- When using cursor down often enough to go to the next month, the focus outline flashes, which I find annoying. We should look into improving the rendering to keep the focused element intact and only change its content. This wasn't an issue in the existing version, since that keeps the focus on the input.
- When using the mouse to select a date, the grid element gets the focus outline briefly, before it fades out. There should be either some explicit delay to highlight the selection before hiding the popup (similar to how Google's material design uses animations to confirm user actions), or the hiding show happen immediately, without any visual changes in the popup, including the focus outline. This also wasn't an issue previously, since the focus stay on the input and the clicked date doesn't change on click.
- Once the grid has focus, tabbing does nothing. Looking at the native datepicker in Chrome, it should be possible to tab from the buttons to the grid and back. And to selects for month/year, when available.
- There's probably more, I'll look at something else for now
On demos/calendar/default.html, once the grid is focused, I can't tab out of it. Somewhat reasonable for the popup datepicker (see item on focus above, though), but just broken for calendar. May need to look at other (native) datepickers to see how that should behave.
demos/calendar/buttonbar.html doesn't work, throws "Uncaught Error: no such method 'valueAsDate' for calendar widget instance"
demos/calendar/dropdown-month-year.html needs to be updated or removed. Spec says: "changeMonth/changeYear: Add a demo or create an extension for calendar - remove"
demos/calendar/localization.html is broken, changing the select throws an error, also about valueAsDate
demos/calendar/min-max.html regressed: Previously the prev/next button got disabled once the min or max range was reached, so in this demo, both buttons should be disabled. If this behaviour is intentional, it needs to be added to the spec.
On demos/calendar/multiple-months.html:
- renders "ui-corner-left" for the second month header, this one shouldn't have any corner classes
- Cursor navigation renders selection on all three months
- Using cursor down to go to the next month advances just once month, not three (like the buttons). It also moves focus from the first to the third grid.
- Tabbing from the prev button focuses the first grid, instead of the next button, like it does with other calendars
- Demo still sets showButtonPanel option, doesn't exist anymore
demos/calendar/other-months.html changes the month when clicking a date from a different month. Current inline datepicker doesn't do that. Spec doesn't mention the behaviour.
demos/datepicker/alt-field.html has an outdated demo description, still mentions options that don't exist anymore. Do we even need this demo? Upgrade guide should mention the removed options, but I don't see the point of demoing this.
demos/datepicker/icon-trigger.html: Description needs an update. The icon can't be tabbed to, making it impossible to use the datepicker with only the keyboard. Adding tabindex="0" should help.
demos/datepicker/localization.html is very much the same as demos/calendar/localization.html. Do we really need both?

I think that's enough for now. I don't care if any of that gets addressed in this PR or later, as long as it gets addressed eventually. The input suppression is definitely a regression in this PR (works fine in datepicker branch), that may be the one thing to address here.

scottgonzalez · 2014-11-21T17:55:20Z

ui/calendar.js

+			"./core",
+			"./widget",
+			"./button"
+		], factory );


Missing dependency for $.date.

And Globalize, see TODO comment. Afaik this is already covered by @rxaviers PR.

https://github.com/jquery/jquery-ui/pull/1341/files#diff-563a5392a566d1a29a68b1b5a37c4cb6R16 adds Globalize ("globalize", "globalize/date") and $.date ("./calendar/date").

fnagel · 2014-11-21T19:50:56Z

Hey @jzaefferer, thanks for the feedback.

Some demos are broken due to the valueAsDate method removal. I will fix this as soon as Scott agreed with those changes.
Some other demo related issues are already on the todo list
Some of your usability related comments are already recorded on the wiki page (todo and "Open issues being discussed" in general)
alt-field.html demo should already be removed. Seems like it was reintroduced in some rebase commit.
Regarding the localization demo: the datepicker localization demo demonstrates how to handle the input value itself. Not sure what to do.

The input suppression is definitely a regression in this PR (works fine in datepicker branch), that may be the one thing to address here.

Broken in Chrome and IE but Firefox works just fine. I will take a look.

I will make sure to recheck your comment when I start working on the associated to do list issues.

fnagel · 2014-11-21T23:35:04Z

Removed altField demo and fixed the input suppression in Chrome and IE.

scottgonzalez · 2014-11-26T15:00:18Z

We should keep valueAsDate() even if it's duplicative. This keeps the API consistent for calendar, datepicker, and native inputs.

fnagel · 2014-11-30T22:55:43Z

@scottgonzalez I've restored valueAsDate but kept the option method changes. Please take a look.

scottgonzalez · 2014-12-13T13:20:18Z

ui/datepicker.js

-		// TODO: This assumes focus is on the first grid. For multi pickers, the widget needs
-		// to maintain the currently focused grid and base queries like this off of it.
-		$( this.picker ).find( ".ui-state-focus" ).not( ":first" ).removeClass( "ui-state-focus" );
+		this.calendarInstance._setOption( "value", this._getParsedValue() );


Why are you using an internal method from another widget? Use the public option() method.

Changed in all places.

Add calendar widget by copying and renaming datepicker widget files. Remove datepicker functionality, options and methods from Calendar. Remove calendar functionality, options and methods from Datepicker. Adjust tests due to split and changed specification. Remove duplicated demo files and fix some demos. Simplify calendar generation, use CSS instead of inline styles. Fix destroy method. Make use of uniqueId method. Fix focus highlighting when month is changed. Add version property. Add common unit tests. Fix input keyboard handling.

Improve render day cell mechanism.

Remove shortcut for closing the calendar and erasing the date (CTRL+END). Remove unwanted CTRL+HOME shortcut and support for enter key on input.

Several minor code improvements and make suppressExpandOnFocus an internal variable, remove partial button widget usage in header

Add Test if these options are set on the underlying calendar instance

Prevents refresh from being called multiple times.

fnagel · 2014-12-28T19:02:45Z

Updated as suggested by Scott.

scottgonzalez · 2015-01-05T16:07:15Z

@jzaefferer Do you want to go through your list again and make sure everything was either addressed or can wait and is on the todo list?

jzaefferer · 2015-01-07T11:41:39Z

I went through my list above and added checkboxes. I think all the regressions are addressed and this PR can land. I've merged the remaining items into the todo list on the wiki.

scottgonzalez · 2015-01-12T14:59:20Z

demos/calendar/buttonbar.html

+	$(function() {
+		$( "#calendar" ).calendar({
+			buttons: {
+				"Today": function() {


Drop the quotes.

scottgonzalez · 2015-01-12T15:10:05Z

A few minor comments, they can just be addressed in a new commit if you want. I think we're good to merge now. We can do any further cleanup after the merge.

fnagel · 2015-01-18T18:28:13Z

@scottgonzalez I've cleaned up the demos. Happy merging :-D

Datepicker: Introduce value option

738a95e

Change status caching, fix existing value related methods, introduce $.date construction with date object, selected property is null by default, add selected getter

fnagel mentioned this pull request Oct 22, 2014

Calendar rebase (3) #1352

Closed

fnagel changed the title ~~Calendar value pr3~~ Calendar rebase (3) Oct 22, 2014

rxaviers mentioned this pull request Oct 22, 2014

Datepicker using Globalize 1.x #1341

Merged

8 tasks

fnagel force-pushed the calendar-value-pr3 branch from e784ac0 to f8b2691 Compare November 7, 2014 17:14

fnagel force-pushed the calendar-value-pr3 branch from f8b2691 to c40c370 Compare November 7, 2014 17:44

scottgonzalez reviewed Nov 21, 2014
View reviewed changes

fnagel force-pushed the calendar-value-pr3 branch from c40c370 to 0cfc479 Compare November 21, 2014 23:35

fnagel force-pushed the calendar-value-pr3 branch from 0cfc479 to a68d714 Compare November 30, 2014 22:55

scottgonzalez reviewed Dec 13, 2014
View reviewed changes

fnagel added 10 commits December 16, 2014 23:02

Calendar: Add min and max option.

ca654c5

Improve render day cell mechanism.

Calendar: Remove select callback reference

54b6e2f

Datepicker: Code clean up for events

56744d0

Datepicker: Remove unwanted keyboard shortcut

f3b058a

Remove shortcut for closing the calendar and erasing the date (CTRL+END). Remove unwanted CTRL+HOME shortcut and support for enter key on input.

Datepicker: Simplify usage of calendar options and avoid duplications

730cde5

Datepicker tests: Add open and close unit tests

7e87b53

Datepicker: Improve document click event

4fe8c4c

Calendar: Fix broken day table cell attributes

f590a43

Datepicker tests: Rewrite event unit tests

dd0bd1a

fnagel added 12 commits December 16, 2014 23:02

Calendar: Fix multiple calendar styles

dbdc846

Calendar: Fix German localization

e1478cc

Datepicker: Fix localization demo

45bc3ec

Datepicker: Several minor code improvements

0b54ad1

Several minor code improvements and make suppressExpandOnFocus an internal variable, remove partial button widget usage in header

Calendar: Fix hover event setting and removing

f5b569d

Datepicker: Improve localization handling, code style

4d9c598

Calendar: Add buttons option

d4ee2bc

Calendar: Adjust files to match reorganization of external directory

5c168fb

Datepicker tests: Remove pass-through options unit tests

da2b9ae

Add Test if these options are set on the underlying calendar instance

Calendar: Use _on for link hover events

ad38ad4

Datepicker: Add missing handling for disabled option

49c0102

Calendar: Remove option method overwrite

73c1d18

fnagel changed the title ~~Calendar rebase (3)~~ Calendar rebase (4) Dec 16, 2014

fnagel added 2 commits December 28, 2014 19:45

Calendar: Make use of _setOptions to call refresh

565a9e1

Prevents refresh from being called multiple times.

Calendar: Sanitize active descendant id selector

2309f4d

fnagel force-pushed the calendar-value-pr3 branch from a68d714 to 2309f4d Compare December 28, 2014 19:01

scottgonzalez reviewed Jan 12, 2015
View reviewed changes

Calendar: Clean up demos.

efe643e

fnagel merged commit efe643e into jquery:datepicker Jan 19, 2015

fnagel deleted the calendar-value-pr3 branch August 25, 2015 15:42

Calendar rebase (4) #1371

Calendar rebase (4) #1371

Uh oh!

Conversation

fnagel commented Oct 22, 2014

Uh oh!

fnagel commented Nov 7, 2014

Uh oh!

fnagel commented Nov 7, 2014

Uh oh!

jzaefferer commented Nov 20, 2014

Uh oh!

scottgonzalez Nov 21, 2014

Choose a reason for hiding this comment

Uh oh!

fnagel Nov 21, 2014

Choose a reason for hiding this comment

Uh oh!

rxaviers Nov 24, 2014

Choose a reason for hiding this comment

Uh oh!

fnagel commented Nov 21, 2014

Uh oh!

fnagel commented Nov 21, 2014

Uh oh!

scottgonzalez commented Nov 26, 2014

Uh oh!

fnagel commented Nov 30, 2014

Uh oh!

scottgonzalez Dec 13, 2014

Choose a reason for hiding this comment

Uh oh!

fnagel Dec 16, 2014

Choose a reason for hiding this comment

Uh oh!

fnagel commented Dec 28, 2014

Uh oh!

scottgonzalez commented Jan 5, 2015

Uh oh!

jzaefferer commented Jan 7, 2015

Uh oh!

scottgonzalez Jan 12, 2015

Choose a reason for hiding this comment

Uh oh!

fnagel Jan 18, 2015

Choose a reason for hiding this comment

Uh oh!

scottgonzalez commented Jan 12, 2015

Uh oh!

fnagel commented Jan 18, 2015

Uh oh!

Uh oh!