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

Remove jQuery calendar from DateField #6626

Closed
24 of 27 tasks
chillu opened this issue Feb 15, 2017 · 7 comments
Closed
24 of 27 tasks

Remove jQuery calendar from DateField #6626

chillu opened this issue Feb 15, 2017 · 7 comments

Comments

@chillu
Copy link
Member

chillu commented Feb 15, 2017

Followup from #6194, where @tractorcow has refactored DateField->setConfig('showcalendar', true) to DateField->setShowCalendar(true). This triggered a discussion whether the PHP API should care about these view aspects, in particular around the "sub views" implemented in DateField to support the CMS use case (DateField_View_JQuery). Since the introduction of this functionality, we've gained a HTML5 date type which comes with built-in, localised date pickers in modern browsers like Chrome, IE Edge and Safari (supported browser overview). Given we want to remove jQuery UI from core in favour of React components, this was a good point to revisit the client-side library use as well.

Merge Process

Tasks

  • Remove DateField_View_JQuery and jQuery datepicker related logic
  • Remove DateField.js wrapper for jQuery UI
  • Remove DateField->setShowCalendar()
  • Add DateField->setHTML5() (consistent with NumericField->setHTML5())
  • Document the limitations of DateField->setLocale() and setFormat() (will be overruled by browser on presentation on setHTML5(true)). Throw exception if locale or format isn't null or ISO on setHTML5(true)
  • Force format to ISO on setHTML5(true)
  • Point to type=date polyfill if CMS users are stuck on older browsers (https://github.com/jonstipe/date-polyfill) (building a MomentJS polyfill instead)
  • Replicate logic to DatetimeField (with type=datetime-local) (nice to have)
  • Replicate logic to TimeField (with type=time)
  • Add default placeholder to DateField documenting date ordering for usability (if type=date isn’t supported)
  • Make scaffolded and core DateField use HTML5 by default
  • Pass through min/max as properties on DateField
  • Rebuild jQuery bundle to exclude datepicker (if easy)
  • Remove jQuery localisation files from repo (only used by datepicker)
  • Use short date formats as fallback input (e.g. 31/12/2017, no spelled out month names)
  • Don't use PHP's ICU formats (since it's hard to convert to a JS library compat format)
  • Use a minimal Modernizr include to determine HTML5 support on client
  • Use Moment.js to format server-provided ISO format into a localised form (Moment.js supports dozens of locales)
  • Use Moment.js to accept localised user input, and convert it to ISO date format in a hidden field (server doesn't need to do conversion)
  • Set a placeholder with an unambiguous formatted string via JS ("Example: 31/12/2017")
  • Validate user input through Moment.js Didn't have client-side validation beforehand (with or without datepicker), let's stick to server-side for the polyfill
  • Remove date/time format customisation in admin/myprofile: It no longer applies for a large amount of use cases (date fields for 85% of users on HTML5 browsers). It means readonly dates (e.g. page history view) will use the default for your locale, but that's acceptable. Admins could still override the locale format globally for all users through YAML config.

Resources:

@tractorcow
Copy link
Contributor

👍 from me. Using html5 date input is preferable in this day and age, especially where mobile browsers might break legacy JS libraries.

@tractorcow
Copy link
Contributor

tractorcow commented Mar 29, 2017

I'd like to suggest some adjustmens @chillu .

I might argue to simply force datefield to html5, and rely on front-end technology to provide the polyfill. The reason I don't suggest this for numericfield is that numericfield does not localise numbers in html5 mode, whereas datefield does (in most browsers). non-html5 behaviour requires a huge amount of code to ensure it works properly, and it would be prudent to trim this down to the minimum. It also complicates the "nonhtml5 / html5" forms that react needs to support when porting this functionality to formschema.

This story should be addressed prior to #6140 as a blocker.

@chillu
Copy link
Member Author

chillu commented Mar 30, 2017

I've thought about this a bit more, and think we can create an acceptable user experience without pulling in a whole bunch of code for a date picker UI - I'm hoping to get rid of jQuery UI in the lifespan of 4.x.

Additional research:

  • Current uses of jQuery UI: Non-React tabs, toggle fields. We've removed the need for jQuery UI Dialogs (all React now), and JSTree never needed it. It seems possible to phase out jQuery UI, unless we commit to it in 4.x through date polyfills. Changes to browser support would be considered an API breakages, so we're effectively supporting IE10 without HTML5 date support until 2020.
  • With our current bundling strategy, 85% of HTML5 compatible browser will receive an extra 70kb of jQuery UI even though they don't need it. Splitting this out would require server-side user agent checks.
  • There's an ECMA Standard DateFormatter API, but it doesn't use ICU date/time placeholders
  • There's no solid approach for parsing PHP's ICU dates in JavaScript (e.g. moment ICU format moment/moment#2050)
  • Polyfills not using jQuery UI (https://github.com/chemerisuk/better-dateinput-polyfill, https://github.com/brianblakely/nodep-date-input-polyfill) have more limited date localisations (<10 locales), but that's not really a large factor
  • Polyfills not using jQuery UI are likely not accessible (WCAG), but users with screen readers aren't likely to use IE10/IE11
  • The only jQuery UI based polyfill is five years old, single maintainer: https://github.com/jonstipe/date-polyfill. We might have to write our own one...
  • Moment.js (without localisation) plus Modernizr is 20kb
  • We probably need Moment.js for client-side localisation soon anyway (e.g. nicely formatted readonly dates in history views)

Suggested additional tasks:

  • Use short date formats as fallback input (e.g. 31/12/2017, no spelled out month names)
  • Don't use PHP's ICU formats (since it's hard to convert to a JS library compat format)
  • Use a minimal Modernizr include to determine HTML5 support on client
  • Use Moment.js to format server-provided ISO format into a localised form (Moment.js supports dozens of locales)
  • Use Moment.js to accept localised user input, and convert it to ISO date format in a hidden field (server doesn't need to do conversion)
  • Set a placeholder with an unambiguous formatted string via JS ("Example: 31/12/2017")
  • Validate user input through Moment.js
  • Remove date/time format customisation in admin/myprofile: It no longer applies for a large amount of use cases (date fields for 85% of users on HTML5 browsers). It means readonly dates (e.g. page history view) will use the default for your locale, but that's acceptable. Admins could still override the locale format globally for all users through YAML config.

@sminnee
Copy link
Member

sminnee commented Mar 30, 2017

👍 I'm comfortable that we take this approach for beta1 and then user test the result. It'll mean that a lot of users (IE11, Firefox, Safari) won't get a date picker, but we don't need to pre-judge the impact of this — that's what user testing is for.

chillu added a commit to open-sausages/silverstripe-framework that referenced this issue Mar 30, 2017
chillu added a commit to open-sausages/silverstripe-admin that referenced this issue Mar 30, 2017
@chillu chillu self-assigned this Mar 31, 2017
@chillu chillu modified the milestones: CMS 4.0.0-alpha7, CMS 4.0.0-beta1 Mar 31, 2017
chillu added a commit to open-sausages/silverstripe-cms that referenced this issue Mar 31, 2017
chillu added a commit to open-sausages/silverstripe-admin that referenced this issue Mar 31, 2017
@chillu
Copy link
Member Author

chillu commented Apr 1, 2017

Client-side validation isn't (easily) supported in the Entwine-based CMS, so it's overkill to invest into this - let's only do client-side validation in the new React-based stuff. HTML5 browser validation also doesn't kick in because we prevent the native form submission via a return false.

@chillu
Copy link
Member Author

chillu commented Apr 1, 2017

Test setup:

  • Set my OSX to a German date format (d.m.y), as well as a 12h and 24h time format
  • Modified frameworktest to show variations with and without HTML5 (pull request).
  • Used the browserstack US defaults (m/d/y)
  • Set Member.Locale to German in the CMS

Confirmed HTML5 operation:

  • Google Chrome Latest on OSX
  • Opera Latest on Windows 10
  • IE Edge on Windows 10
  • iPad Pro on iOS 10

Confirmed non-HTML5 operation:

  • IE11 on Windows 10
  • Firefox Latest on OSX
  • Safari Latest on OSX

chillu added a commit to open-sausages/silverstripe-admin that referenced this issue Apr 1, 2017
chillu added a commit to open-sausages/silverstripe-cms that referenced this issue Apr 3, 2017
chillu added a commit to open-sausages/silverstripe-admin that referenced this issue Apr 3, 2017
chillu added a commit to open-sausages/silverstripe-testsession that referenced this issue Apr 3, 2017
@tractorcow
Copy link
Contributor

OK, everything is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants