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

[D8][DX] Add Modernizr to core. #1009

Open
klonos opened this issue Jun 14, 2015 · 5 comments
Open

[D8][DX] Add Modernizr to core. #1009

klonos opened this issue Jun 14, 2015 · 5 comments

Comments

@klonos
Copy link
Member

klonos commented Jun 14, 2015

This is the same issue as https://www.drupal.org/node/1252178 for D8. This request is not simply in order to bring us in feature parity with D8 - it's been mentioned in various issues like #189, #214, #801 and latest in #988. It was listed in #211 but dropped as N/A back then. Reconsider?

@wesruv
Copy link
Member

wesruv commented Jun 15, 2015

Step 1) someone should get it in contrib :)

@quicksketch
Copy link
Member

D8 decided to use a custom build of Modernizr that only included a few key feature detections:

  • inputtypes: Detects HTML5 elements such as tel, email, number.
  • touch: Detects touch screen support.
  • elem_details: Detects the HTML5 "details" element.

And some minor APIs:

  • cssclasses: Adds classes to the body tag based on detections.
  • addtest: Extend modernizr's current test list. docs
  • teststyles: docs
  • prefixes: Provides the list of browser vendor prefixes docs

The modular aspect of modernizr means that individual themes could provide additional detections for things they needed specifically. Of note, the most recent place we ran across this was for flexbox detection (#988), which D8 did not include in their out-of-box configuration. As the place we would need this is in the presentation of themes (system.module) and layouts (layout.module), we'd need to include flexbox detection as an additional core item, rather than being in a specific theme like Seven.

One thing that has me a bit worried is that Modernizr 3.0 seems to be on the cusp of release. We may be better off in the long run if we write detections where necessary in the interim, and add modernizr after the release.

That said, I'm also not opposed to just leaving our own detections in place once we have them, as usually they're short and effective. e.g. when we ported https://www.drupal.org/node/1261002, we just wrote our own touch detection instead of using Modernizr (see backdrop/backdrop#312):

  // Touch support, borrowed from Modernizer.touch.
  this.touchSupport = ('ontouchstart' in window) || window.DocumentTouch && document instanceof DocumentTouch;

@klonos
Copy link
Member Author

klonos commented Nov 29, 2015

This is also part of #378

@jenlampton jenlampton changed the title [TX] Add Modernizr to core. [DX] Add Modernizr to core. Apr 27, 2019
@jenlampton jenlampton changed the title [DX] Add Modernizr to core. [D8][DX] Add Modernizr to core. Apr 28, 2019
@klonos
Copy link
Member Author

klonos commented Oct 13, 2019

Pinging @domaingood since he mentioned this on Gitter 😉

@klonos
Copy link
Member Author

klonos commented Aug 9, 2022

FTR: https://www.drupal.org/project/drupal/releases/9.4.0 (change record: Modernizr touchevents test deprecated)

The Modernizr.touchevents browser feature detection check has been deprecated. In addition, Modernizr is no longer responsible within Drupal for detecting touch devices and adding the touchevents/no-touchevents classes to the HTML element based on the detection result. The specific functionality of adding classes based on touch device detection is now provided via a core library: core/drupal.touchevents-test. The detection method used is identical to Modernizr's.

https://www.drupal.org/project/drupal/issues/3101922

In #3100937: Update modernizr to 3.8.0, it was discovered that Modernizr's touchevent test is deprecated. Modernizr/Modernizr#2432
There are also problems with that test from Modernizr 3.7.1 onward, which requires an override of that test in Drupal core (added in the aforementioned issue).

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

4 participants