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

Support fauxton on IE10,IE11 and Edge #1013

Merged
merged 9 commits into from
Nov 9, 2017

Conversation

popojargo
Copy link
Member

@popojargo popojargo commented Nov 1, 2017

Overview

  • Added babel-polyfill to support ES6 functions on browsers that doesn't support them
  • Fixed layouts problems on IE11 and Edge

Testing recommendations

Since we seems to mainly support Firefox and Chrome, there are not automated tests for IE.

With my changes, I fixed:

  • EDGE: Fixed JSON API bar display
  • IE11/IE10 : Fixed replication page that wasnt shown
  • IE11/IE10 : We are now able to use React 👍
  • IE11/IE10 : Polling slider where not shown correctly (but they still don't work correctly on IE)

GitHub issue number

#949

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;
  • Update rebar.config.script with the correct tag once a new Fauxton release is made

@Antonio-Maranhao
Copy link
Contributor

@popojargo @garrensmith
Took me some time to get a VM with IE11 but now I'm set. I'll continue looking into this tomorrow.

@@ -20,7 +20,7 @@ const settings = require('./tasks/helper')

module.exports = {
entry: {
bundle: './app/main.js' //Our starting point for our development.
bundle: ['babel-polyfill','./app/main.js'] //Our starting point for our development.
Copy link
Contributor

@Antonio-Maranhao Antonio-Maranhao Nov 2, 2017

Choose a reason for hiding this comment

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

To address a previous comment from @garrensmith we could be more granular using core-js and polyfill only what's needed. I was able to launch the UI using bundle: ['core-js/fn/array', 'core-js/fn/symbol', 'core-js/fn/promise', './app/main.js']

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any idea how I can found which polyfill are missings?

@@ -20,7 +20,7 @@ const settings = require('./tasks/helper')

module.exports = {
entry: {
bundle: './app/main.js' //Our starting point for our development.
bundle: ['babel-polyfill','./app/main.js'] //Our starting point for our development.
Copy link
Contributor

Choose a reason for hiding this comment

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

Side note... webpack.config.release.js needs to be updated as well.

@@ -388,8 +388,13 @@ export class ActiveTasksPollingWidgetController extends React.Component {
state = this.getStoreState();

render() {
const isIE1X = document.documentMode == 11 || document.documentMode == 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you replace with the Helpers.isIE1X() function you created?

@Antonio-Maranhao
Copy link
Contributor

Btw... @popojargo are you planning on fixing Polling slider as well? If not we should open an issue so we don't lose track of it.

@garrensmith
Copy link
Member

garrensmith commented Nov 2, 2017 via email

@popojargo
Copy link
Member Author

@Antonio-Maranhao As for the polling, I think it could be for another issue. This is mostly a problem with the mouse events in IE and doesn't have any easy fix so far.

@popojargo
Copy link
Member Author

@Antonio-Maranhao I managed to fix the issue for the polling with a package: react-script. The onChange event for range inputs should be fixed on React16(but I can't confirm).

@Antonio-Maranhao
Copy link
Contributor

@garrensmith any concerns about the ISC license for react-range?
https://github.com/mapbox/react-range

@popojargo
Copy link
Member Author

@Antonio-Maranhao ISC is like MIT so it should pass right?

@garrensmith
Copy link
Member

garrensmith commented Nov 7, 2017 via email

Copy link
Member

@garrensmith garrensmith left a comment

Choose a reason for hiding this comment

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

Great job @popojargo this is awesome. I'll merge once the build passes.

@garrensmith garrensmith merged commit 6960440 into apache:master Nov 9, 2017
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.

None yet

3 participants