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

Datasets; part of the IE10 compatibility viacrucis #29

Merged
merged 3 commits into from
Jan 18, 2017

Conversation

gabrielmontagne
Copy link
Contributor

Description

Makes the grid revert to data-whatever-attribute when node.dataset is not available.

Motivation and Context

The grid doesn't pretend to run everywhere -- it's the responsibility of the app in which this is to be embedded to cater for transpilations, perspirations and polyfillings which, we'd guess, are needed not only for this component.

But while most of the modern JS usage can be fixed at build time using something like babel-polyfill, some things remain run time pastimes.

At The Office™️ we had two problems that forced us to lay hands on our pre-compiled code (actually only one main problem, IE10 support, but... )

  1. D3v4 dispatch of native dom events in IE -- we had to add a good old polyfill to our app.
  2. The dataset incompatibility, which cannot really be addressed "once and for all", but needed to be, sad but true, embedded on the grid.

Hence this PR.

Smarter people might find a nicer solution. Please?
Anyways, for now, if you don't mind....

How Was This Tested?

To properly test this we created a (still offline) fully webpacked build with polyfills, etc. to run this on IE.
Nothing we can easily upload to a block

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change follows the style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the contribution guidelines
  • I have added tests to cover my changes
  • All new and existing tests passed

@gabrielmontagne
Copy link
Contributor Author

@grancalavera ... so far like this goes the var.

@Poetro
Copy link
Member

Poetro commented Jan 13, 2017

There are polyfills for ES5 browsers like element-dataset, so I don't think it is necessary to hack around this.

@gabrielmontagne
Copy link
Contributor Author

gabrielmontagne commented Jan 13, 2017 via email

@gabrielmontagne
Copy link
Contributor Author

Yeah, tried that out, even the one you suggested -- but, as I feared, until JS Proxies are supported -- that is, never for good old IE, there is no way that doing node.dataset.heyPeter = 5 can be turned into <node data-hey-peter="5" />

So, I think, we should bring in this ponyfill in until -- ?

@cristiano-belloni
Copy link
Contributor

Also, I'd be wary of bringing in a polyfill using proxies. Might be my superstition, but I heard they're slow as hell (is hell slow?)

Copy link
Contributor

@cristiano-belloni cristiano-belloni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gabrielmontagne
Copy link
Contributor Author

hell is slow; I've seen it: F12 => Emulation => 10

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

Successfully merging this pull request may close these issues.

3 participants