feature: support ie browser #1887

SmileLifeIven · 2021-11-25T10:25:38Z

Description

Please explain the changes you made here. Include an example of what your changes fix or how to use the changes.

Checklist

Created tests which fail without the change (if possible)
All tests passing (yarn test)
Extended the README / documentation, if necessary

adumesny

thanks for making it work, although I had no interest in supporting a long dead and no longer supported browser... ok if it comes free and only IE user need to include polyfill.

adumesny · 2021-11-25T20:59:33Z

babel.config.js

@@ -0,0 +1,34 @@
+module.exports = function (api) {


we don't use babel for building this lib (maybe we should ?) so please remove.

adumesny · 2021-11-25T21:00:13Z

demo/web1.html

  <script type="module" src="https://unpkg.com/ionicons@4.5.10-0/dist/ionicons/ionicons.esm.js"></script>
  <script nomodule="" src="https://unpkg.com/ionicons@4.5.10-0/dist/ionicons/ionicons.js"></script>
-
+  <script src="../src/gridstack-poly.js"></script>


should be ../dist as poly should be copies to the build folder as well (or needs to)

adumesny · 2021-11-25T21:01:38Z

package.json

    "connect": "^3.7.0",
    "core-js": "^3.6.4",
-    "coveralls": "^3.0.9",
-    "doctoc": "^1.4.0",
+    "coveralls": "^2.11.4",


why did you use an older one ?

adumesny · 2021-11-25T21:02:04Z

package.json

@@ -55,15 +55,20 @@
  },
  "homepage": "http://gridstack.github.io/gridstack.js/",
  "devDependencies": {
+    "@babel/plugin-proposal-class-properties": "^7.16.0",


remove all babel please.

adumesny · 2021-11-25T21:03:54Z

package.json

    "webpack-cli": "^4.6.0"
+  },
+  "dependencies": {
+    "yarn": "^1.22.17"


should be a devDependencies, not dependency (that user need to install)

also yarn.lock should be updated instead... not sure why you had (?) to change the rev of the libs...

adumesny · 2021-11-25T21:04:57Z

src/gridstack.ts

@@ -5,7 +5,7 @@
 * Copyright (c) 2021 Alain Dumesny
 * see root license https://github.com/gridstack/gridstack.js/tree/master/LICENSE
 */
-
+require('@babel/polyfill')


no, I don't want modern browsers to compile poyfill. Only the few IE users can include our polyfill js file instead...

adumesny · 2021-11-25T21:07:20Z

src/utils.ts

@@ -295,7 +295,7 @@ export class Utils {

  /** @internal returns the passed element if scrollable, else the closest parent that will, up to the entire document scrolling element */
  static getScrollElement(el?: HTMLElement): HTMLElement {
-    if (!el) return document.scrollingElement as HTMLElement;
+    if (!el) return document.documentElement as HTMLElement;


need to understand that change as I don't want break other browser for IE, which has been dead for a while really.. so not interested in supporting it unless it's 'free'...

maybe do || if scrollingElement doesn't exist on IE ?

adumesny · 2021-11-25T21:08:01Z

webpack.config.js

-  devtool: 'source-map',
-  // devtool: 'eval-source-map', // for best (large .js) debugging. see https://survivejs.com/webpack/building/source-maps/
+  // devtool: 'source-map',
+  devtool: 'eval-source-map', // for best (large .js) debugging. see https://survivejs.com/webpack/building/source-maps/


can't check that in!

adumesny · 2021-11-25T21:09:00Z

webpack.config.js

-        use: 'ts-loader',
+        use: [
+          {
+            loader: 'babel-loader',


no babel please, already have webpack and ts compiler... don't think we need that many packages!

SmileLifeIven · 2021-11-28T13:24:33Z

Sorry for your time, Those are my mistakes.

adumesny

thanks, looks much better. couple comments

adumesny · 2021-11-28T16:40:54Z

src/gridstack-poly.js

+// https://developer.mozilla.org/zh-CN/docs/Web/API/CustomEvent/CustomEvent
+(function () {
+  if (window.navigator.userAgent.indexOf('Chrome') > -1 ) {


did you actually get that from core-js polyfill ? checking for specific browser type seems bridle to me...

adumesny · 2021-11-28T16:44:45Z

src/gridstack-poly.js

@@ -146,6 +146,23 @@ if (!Array.prototype.findIndex) {
  });
 })([Element.prototype, Document.prototype, DocumentFragment.prototype]);

+// from: https://github.com/jserz/js_piece/blob/master/DOM/Element/prepend()/prepend().md


can't access that URL... looks like a private repo. isn't there a developer.mozilla.org version instead ?

adumesny · 2021-11-28T16:45:55Z

src/gridstack-poly.js

+}
+
+// https://developer.mozilla.org/zh-CN/docs/Web/API/CustomEvent/CustomEvent


small request, can you use en-US in urls instead ? more people read english doc. thank you.

adumesny · 2021-11-28T16:46:31Z

src/gridstack-poly.js

+})();
+
+//https://github.com/jserz/js_piece/blob/master/DOM/ChildNode/remove()/remove().md


same, can't access URL

adumesny · 2021-11-28T16:48:01Z

src/gridstack-poly.js

+// suport IE
+(function () {
+  if (!document.scrollingElement) {


interesting way to support field name change... is there a problem caching this though ? I'm ok with caller doing
document.scrollingElement || document.documentElement
instead.

adumesny · 2021-11-28T16:49:25Z

src/gridstack.ts

@@ -164,7 +163,7 @@ export class GridStack {
    // create the grid element, but check if the passed 'parent' already has grid styling and should be used instead
    let el = parent;
    if (!parent.classList.contains('grid-stack')) {
-      let doc = document.implementation.createHTMLDocument();
+      let doc = document.implementation.createHTMLDocument("");


use '' (2 single quote) instead, also you didn't answer me if IE require a param to be passed (if so add comment)

https://developer.mozilla.org/en-US/docs/Web/API/DOMImplementation/createHTMLDocument
title Optional (except in IE)
I see that now....

adumesny · 2021-11-28T16:50:59Z

tsconfig.json

@@ -15,7 +15,8 @@
    "outDir": "./dist",
    "sourceMap": true,
    "strict": false,
-    "target": "ES6"
+    // "target": "ES6",
+    "target": "ES5"


what is the size difference between ES5 and 6 (which I believe was smaller) ? is that required for IE. need to trade off all browser vs obsolete IE

adumesny · 2021-11-28T16:51:21Z

webpack.config.js

@@ -9,6 +9,7 @@ module.exports = {
  mode: 'production', // production vs development
  devtool: 'source-map',
  // devtool: 'eval-source-map', // for best (large .js) debugging. see https://survivejs.com/webpack/building/source-maps/
+  target: ['web', 'es5'],


what does that do ?

…l for CustomEvent.

adumesny · 2021-11-30T15:37:58Z

you still have not answer my question on the size difference between ES5 and ES6 I had, and what the web target is for... and please add URL to polyfill code you add so we know where it is coming from...
looks good otherwise.

SmileLifeIven · 2021-12-01T01:14:12Z

you still have not answer my question on the size difference between ES5 and ES6 I had, and what the web target is for... and please add URL to polyfill code you add so we know where it is coming from... looks good otherwise.

Size:
ES5: gridstack-jq.js -> 202KB gridstack-h5.js -> 81KB gridstack-static.js -> 1KB
ES6: gridstack-jq.js -> 196KB gridstack-h5.js -> 71KB gridstack-static.js -> 40KB

Web target is for some old system with original browser, they want to use new feature with they system, and there is only IE browser when some customer install system, we shoule support the feature instead of updating customer's browser.

And the polyfill code source url I will add it.

SmileLifeIven · 2021-12-01T01:58:41Z

Or we could add new script to build for es5 and relative config add into new filefolder.

adumesny · 2021-12-01T16:04:49Z

| gridstack-static.js -> 1KB
typo...

| Or we could add new script to build for es5 and relative config add into new filefolder.

Yes, I think that would be better, because 10k for gridstack-h5.js @ 71KB is a big difference and very few if any need IE support, so I don't want to do that. create an es5 sub-folder for it

adumesny · 2021-12-03T15:25:05Z

re-opening in case someone else wants to continue this effort...

SmileLifeIven · 2021-12-06T05:52:07Z

Done. Add a es5 filefolder.

adumesny

almost there! thank you.

es5/tsconfig.json

es5/webpack.config.js

package.json

SmileLifeIven · 2021-12-07T11:31:17Z

Check it.

adumesny · 2021-12-07T15:15:58Z

thank you! I will verify it and update the documentation and publish a new version in the next couple days...

SmileLifeIven · 2021-12-08T00:58:28Z

😊

* some cleanup and documendation for gridstack#1887 for IE

feature: support ie browser

0e5c746

adumesny requested changes Nov 25, 2021

View reviewed changes

SmileLifeIven added 2 commits November 28, 2021 21:03

fix: remove babel

27713d8

fix: remove unnecessary change

edb0b28

adumesny requested changes Nov 28, 2021

View reviewed changes

fix: add alternative option for scrollingElement. use offical polyfil…

3900bf9

…l for CustomEvent.

SmileLifeIven requested a review from adumesny November 30, 2021 06:31

fix: add poly code url for custome event

a0b3d74

SmileLifeIven closed this Dec 3, 2021

adumesny reopened this Dec 3, 2021

feature: add es5 build script

0324e2c

adumesny requested changes Dec 6, 2021

View reviewed changes

es5/tsconfig.json Outdated Show resolved Hide resolved

es5/webpack.config.js Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

feature: update es5 filefolder path

2e6d58c

adumesny merged commit e7a0ade into gridstack:master Dec 7, 2021

adumesny added a commit to adumesny/gridstack.js that referenced this pull request Dec 10, 2021

more IE support

ace8dca

* some cleanup and documendation for gridstack#1887 for IE

adumesny mentioned this pull request Dec 10, 2021

more IE support #1893

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: support ie browser #1887

feature: support ie browser #1887

SmileLifeIven commented Nov 25, 2021

adumesny left a comment

adumesny Nov 25, 2021

adumesny Nov 25, 2021

SmileLifeIven Nov 26, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

adumesny Nov 25, 2021

SmileLifeIven commented Nov 28, 2021

adumesny left a comment

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny Nov 28, 2021

adumesny commented Nov 30, 2021

SmileLifeIven commented Dec 1, 2021

SmileLifeIven commented Dec 1, 2021

adumesny commented Dec 1, 2021 •

edited

adumesny commented Dec 3, 2021

SmileLifeIven commented Dec 6, 2021

adumesny left a comment

SmileLifeIven commented Dec 7, 2021

adumesny commented Dec 7, 2021

SmileLifeIven commented Dec 8, 2021

		}

		// https://developer.mozilla.org/zh-CN/docs/Web/API/CustomEvent/CustomEvent

		})();

		//https://github.com/jserz/js_piece/blob/master/DOM/ChildNode/remove()/remove().md

feature: support ie browser #1887

feature: support ie browser #1887

Conversation

SmileLifeIven commented Nov 25, 2021

Description

Checklist

adumesny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmileLifeIven commented Nov 28, 2021

adumesny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adumesny commented Nov 30, 2021

SmileLifeIven commented Dec 1, 2021

SmileLifeIven commented Dec 1, 2021

adumesny commented Dec 1, 2021 • edited

adumesny commented Dec 3, 2021

SmileLifeIven commented Dec 6, 2021

adumesny left a comment

Choose a reason for hiding this comment

SmileLifeIven commented Dec 7, 2021

adumesny commented Dec 7, 2021

SmileLifeIven commented Dec 8, 2021

adumesny commented Dec 1, 2021 •

edited