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

stop polyfill initialising if there are no date inputs on the page or… #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pixelthing
Copy link

Hi @jcgertig - Loving this polyfill, but noticed that it initialises itself (including dependencies) even if there aren't any input:date on the page, or even if the browser supports it's own datepicker that overrides the polyfill. So this fixes that (if you like it) - it can save maybe 50ms on a modern PC, but can save a lot more on slower phones that would never use it anyway (most versions of android/iOS support native pickers).

Attached are flamecharts of how the existing version looks in Chrome 58 (which supports datepicker and overrides the polyfill anyway), how it looks when it detects browsers that support datepickers, and how it looks when there aren't any date inputs on the page. (ignore the time to load localhost HTML, webpack server is a bit inconsistent with load times!).

Hope it's ok to submit a PR!

date-input-polyfill-currently
date-input-polyfill-new-date-supporting-browsers
date-input-polyfill-date-notpresent-on-page

… if the browser already supports a date picker.
@jcgertig
Copy link
Owner

I like the concept but this needs to be optional as not to break backwards compatibility. It loading by default helps sites that are dynamic and build dynamic forms. My suggestion is a data-attribute on the body tag such as data-date-input-conditional-load to decide if it loads by default or not.

Also don't commit changes to the .dist file I will commit that after doing a push to npm.

@pixelthing
Copy link
Author

understood about the commit to the dist - I'll work in the optional characteristics. Thanks!

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.

2 participants