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

Create a message that we don't support IE 11 #954

Closed
zepumph opened this issue Jun 11, 2020 · 54 comments
Closed

Create a message that we don't support IE 11 #954

zepumph opened this issue Jun 11, 2020 · 54 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jun 11, 2020

From developer meeting today we decided to remove support for IE11. But first we will need to make sure that a message displays for IE 11 when running on that browser. It is not currently clear if the message says: "IE is not supported, go away", or more like "IE is not supported, continue and good luck, it will probably fail".

Marking for developer meeting.

@mattpen
Copy link
Contributor

mattpen commented Jun 18, 2020

Current website solution is below. The @supports (display: grid) media query works on all modern browsers and does not work on IE, which is what allows this to work.

<div id="ie11Blocker">
  <div class="modal">
    <p>
      <wicket:message key="ie11-blocking-message">
        The PhET website does not support Internet Explorer. We recommend using a modern web browser such as Chrome,
        Firefox, or Edge.
      </wicket:message>
    </p>
    <button id="ie11Blocker-close">
      <i class="fa fa-times"></i>
    </button>
  </div>

  <script>
    document.getElementById( 'ie11Blocker' ).addEventListener( 'click', function () {
      document.getElementById( 'ie11Blocker' ).style.display = 'none';
    } );
  </script>
</div>
#ie11Blocker {
  display: flex;
  position: fixed;
  top: 0;
  left: 0;
  right: 0;
  height: 100vh;
  width: 100vw;
  background: rgba(0, 0, 0, .3);
  z-index: 10000;
  align-items: center;
}

#ie11Blocker .modal {
  position: relative;
  background: #efefef;
  border-radius: 10px;
  max-width: 410px;
  margin: auto;
  padding: 30px;
  font-size: 20px;
  font-weight: 100;
}

#ie11Blocker-close {
  position: absolute;
  top: 0;
  right: 0;
  background: transparent;
  border: none;
  padding: 5px 10px;
  cursor: pointer;
}

@supports (display: grid) {
  #ie11Blocker {
    display: none !important;
  }
}

@chrisklus
Copy link
Contributor

from 06/18/20 dev meeting:

@mattpen suggested a css option that would show a dialog in IE

Since the sim will definitely crash, there are concerns that not proactively preventing the sim from running on IE would be a bad UX for the user and/or interfere with the dialog shown in css.

@samreid proposed a solution that would use JS to detect IE and return out before trying to run the sim.

We think that some combination of these two things would be nice, such that the sim intentionally doesn't run and the user is prompted with a message. To start off, @ariel-phet is not concerned with how the dialog looks but just wants a simple message like "IE is not supported" in a dialog in the center of the screen as a proof-of-concept.

@chrisklus and @Denz1994 will pair on this, and will aim to have something working by next dev meeting.

@zepumph
Copy link
Member Author

zepumph commented Jun 18, 2020

We also discussed that this needs to be done before the next RC version (we think that will be density).

@chrisklus
Copy link
Contributor

chrisklus commented Jun 22, 2020

Patch from @Denz1994 and me today:

Index: templates/sim.html
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- templates/sim.html	(revision 985a7dbb6ce503898afce625ddba906a11ad7a21)
+++ templates/sim.html	(date 1592849119000)
@@ -21,6 +21,7 @@
 -->
 <!-- body is only made black for the loading phase so that the splash screen is black -->
 <body style="background-color:black;">
+{{PHET_IE_DETECTION}}
 {{PHET_SIM_SCRIPTS}}
 </body>
 </html>
Index: js/grunt/packageRunnable.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/grunt/packageRunnable.js	(revision 985a7dbb6ce503898afce625ddba906a11ad7a21)
+++ js/grunt/packageRunnable.js	(date 1592849275000)
@@ -39,6 +39,8 @@
   assert( typeof locale === 'string', 'Requires locale' );
   assert( typeof htmlHeader === 'string', 'Requires htmlHeader' );
 
+  const simIEDetection = grunt.file.read( '../chipper/templates/sim-ie-detection.html' );
+
   const localizedTitle = stringMap[ locale ][ getTitleStringKey( repo ) ];
 
   // Directory on the PhET website where the latest version of the sim lives
@@ -48,6 +50,7 @@
     PHET_CARRIAGE_RETURN: '\r',
     PHET_SIM_TITLE: encoder.htmlEncode( localizedTitle ),
     PHET_HTML_HEADER: htmlHeader,
+    PHET_IE_DETECTION: simIEDetection,
     PHET_SIM_SCRIPTS: scripts.map( script => `<script type="text/javascript">${script}</script>` ).join( '\n' ),
 
     // metadata for Open Graph protocol, see phet-edmodo#2
Index: templates/sim-development.html
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- templates/sim-development.html	(revision 985a7dbb6ce503898afce625ddba906a11ad7a21)
+++ templates/sim-development.html	(date 1592844008000)
@@ -16,6 +16,8 @@
 {{BODYSTART}}
 
 <script type="text/javascript">
+  
+  console.log( 'hello' );
 
   window.phet = window.phet || {};
   window.phet.chipper = window.phet.chipper || {};
Index: templates/sim-ie-detection.html
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- templates/sim-ie-detection.html	(date 1592851093000)
+++ templates/sim-ie-detection.html	(date 1592851093000)
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+
+<div id="ie-detection">
+
+  <style>
+    #ie-dialog-container {
+      display: flex;
+      position: fixed;
+      top: 0;
+      left: 0;
+      right: 0;
+      height: 100vh;
+      width: 100vw;
+      background: rgba(0, 0, 0, .3);
+      z-index: 10000;
+      align-items: center;
+    }
+
+    #ie-dialog-container .ie-dialog {
+      position: relative;
+      background: #efefef;
+      border-radius: 10px;
+      max-width: 410px;
+      margin: auto;
+      padding: 30px;
+      font-size: 20px;
+      font-weight: 100;
+    }
+
+    @supports (display: grid) {
+      #ie-dialog-container {
+        display: none !important;
+      }
+    }
+  </style>
+
+  <div id="ie-dialog-container">
+    <div class="ie-dialog">
+      <p>Internet Explorer is not supported.</p>
+    </div>
+  </div>
+
+  <script type="text/javascript">
+    var ieDialogContainer = document.getElementById( 'ie-dialog-container' );
+
+    if ( ieDialogContainer.style.display === 'none' ) {
+
+      // remove all remaining scripts if IE is detected
+      var scripts = document.getElementsByTagName( 'script' );
+      for ( var i = scripts.length; i >= 0; i-- ) {
+        if ( scripts[ i ] && scripts[ i ].parentNode && scripts[ i ].parentNode.id !== 'ie-detection' ) {
+          scripts[ i ].parentNode.removeChild( scripts[ i ] );
+          console.log( scripts.length );
+        }
+      }
+    }
+  </script>
+
+</div>
\ No newline at end of file

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 23, 2020

the next RC version (we think that will be density).

Molecules and Light is also ready for an RC out of master and should include this.

@chrisklus
Copy link
Contributor

@Denz1994 and got a script working that uses the same user agent detection that phet-core/platform.js uses. If IE is detected, we set a global flag to true and a guard that was previously added to every other script during build will stop them from running. Thanks to @jonathanolson and @mattpen for assisting us with various approaches for trying to get this working.

We added this logic for the standard html builds, as well as xhtml. All built files appear to be working.

For now, we've just included a simple message in a simple dialog. We can expand the message/styling per additional requests.

image

@terracoda
Copy link
Contributor

We will need to make this message accessible to screen readers and have a link back to the Sim page - or to somewhere useful.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 23, 2020

Thanks @Denz1994 and @chrisklus. Adding these attributes to the HTML in your snippet would essentially match the accessible markup we use in sims. I also like @terracoda idea of adding a link to leave the page.

<div id="ie-dialog-container">
  <div class="ie-dialog" role="dialog" aria-labelledby="dialog-title">
    <h2 id="dialog-title">Warning</h2>
    <p>Internet Explorer is not supported.</p>
    <a>A link, maybe to PhET home page, https://phet.colorado.edu/en/help-center/running-sims, or somehwere else</a>
  </div>
</div>

@terracoda
Copy link
Contributor

Should we suggest the supported browsers? Or should we link to the systems support page where what we support is listed?

@terracoda
Copy link
Contributor

I see there is no Systems requirement page. The info is on the sim pages.

I would suggest:

<div id="ie-dialog-container">
  <div class="ie-dialog" role="dialog" aria-labelledby="dialog-title">
    <h2 id="dialog-title">Platform Warning</h2>
    <p>Internet Explorer is not supported.</p>
    <p>Please try a different browser or see Systems Requirements on any Simulation Page. <a href="https://phet.colorado.edu/en/simulations/browse">Browse SImulations</a></a>
  </div>
</div>

Alternatively, you could link back to the simulation page for the sim they are trying to load, or link to the home page.

Also, if you don't want to provide a link, you could add a close button, that closes the dialog and returns to the simulation page. That might actually be better.

<div id="ie-dialog-container">
  <div class="ie-dialog" role="dialog" aria-labelledby="dialog-title">
    <h2 id="dialog-title">Platform Warning</h2>
    <p>Internet Explorer is not supported.</p>
    <p>Please try a different browser or see Systems Requirements on Simulation Page.</p>
     <button>Close</button>
  </div>
</div>

Focus should be placed on the close button on dialog load.

@ariel-phet
Copy link
Contributor

@amanda-phet now that this basic concept is working, could you make a mockup of a simple dialog for us to use in this case (I suggest reading through the entire issue for context). It can be very simply styled, but would appreciate your thoughts on what it might look like.

@amanda-phet
Copy link

We have system requirements in the help center and could create a new help center entry about IE specifically, or link to this entry about HTML5 sims https://phet.colorado.edu/en/help-center/running-sims/general#q11-header.

image

@terracoda
Copy link
Contributor

Nice @amanda-phet, could you use a warning icon that is more rounded - kind of in the same style as the green checkbox in this dialog?
Screen Shot 2020-06-24 at 12 41 22 PM.

Maybe something like this:
Screen Shot 2020-06-24 at 12 45 05 PM

The icon could be placed left of the heading if you think that would work.

[close-icon] (visually right aligned, of course)

[warning-icon] Platform Warning
Internet Explorer is not supported.
Please try a different browser or see our {{system requirements}}.

@samreid
Copy link
Member

samreid commented Jun 24, 2020

Will we want to generalize or repurpose this dialog for other warnings? For instance, should sims that require WebGL leverage this feature?

@amanda-phet
Copy link

@terracoda the tricky thing about putting the icon in the heading is that the heading could run into the close icon, so I want to be sure there is ample room. (I'm assuming this will be translatable).

image

If we want it to look like the up-to-date dialog, I'd recommend removing the title.
image

@terracoda
Copy link
Contributor

Removing the title works for me.

@chrisklus
Copy link
Contributor

If we want it to look like the up-to-date dialog, I'd recommend removing the title.

That layout looks strange to me since there is text below it but not above it (the up-to-date dialog is more balanced). I prefer the first one, or playing with the layout of the second one a bit more if we like the title removed. Maybe "Internet Explorer is not supported" is more vertically centered and the "Please try..." text is a smaller footer.

@chrisklus
Copy link
Contributor

@Denz1994 and I tested this morning and the IE error page worked as expected on .html versions after my recent changes. We also confirmed that the xhtml script crashed before the error page appeared, as @zepumph and I mentioned may happen in #963 (comment), as per the reasoning from #954 (comment). The script crashed because of arrow function syntax from much later in the code than the IE detection code.

In the above commit, I loaded the initialization script for the xhtml version separately from the main sim script, so hopefully this fixes the problem. @Denz1994 and I are going to test again later today.

@chrisklus
Copy link
Contributor

chrisklus commented Aug 7, 2020

@Denz1994 and I just finished testing and the xhtml version is indeed now displaying the error page. Thanks!

image

@zepumph would you be interested in reviewing this again? It would consist of checking my thoughts from #954 (comment), and looking at commits 576fff5 and 381be90.

@chrisklus chrisklus assigned zepumph and unassigned chrisklus Aug 7, 2020
@zepumph
Copy link
Member Author

zepumph commented Aug 7, 2020

  • 381be90 creates a discrepancy between packageRunnable.js and packageXHTML.js which isn't totally clear as to why it exists. This code split adds a bit of a maintainability cost to the build process. Here are some potential solutions.
    • The least costly option is to describe why there is a specific initializationScript option to packageXHTML, getting specific about how we need the IE11 warning dialog to run separately.
    • A more expensive option is to make things consistent by separating out all the initialization scripts (I don't see a real benefit to this one).
    • The most explicit thing to do, which is also more work than is necessarily needed, would be to separate out the IE dialog into its own file, and add that as a separate variable to packageXHTML. Then it is clear that it isn't actually the general initialization that needs to go first, just the IE dialog code.

I think I prefer the last option, but don't mind adding the doc string if you feel strongly.

Instead, I just converted back to ES5 in the source.

That seems really reasonable to me. That said it is a bit hidden, and I worry that at some point someone may add es6 to that file without realizing it. The following doc string does not feel loud enough to keep me confident that es6 will stay out of this file indefinitely:

// IE error page. Note: this is not transpiled and needs to support IE.

This may be another argument for pulling the ie dialog out into its own js file, and adding it to the list of scripts to packageHTML and in a separate spot in packageXHTML. My hope is that when this issue is closed, we never have to think about IE again.

  • Is QA going to run a sim on IE11 every RC or something just to make sure this dialog works?

The testing I did:

  • I built a sim from master
  • I loaded the XHTML in IE and got the dialog, along with one console error (seems right to me!)
  • I loaded the en html in IE and got the dialog, along with many errors (likely one per script after the IE dialog script)
    This is looking really nice! Thanks for all the hard work on it.

@zepumph zepumph assigned chrisklus and unassigned zepumph Aug 7, 2020
@jessegreenberg
Copy link
Contributor

We reviewed this during dev meeting 8/13/20 and discussed that this no longer needs to block publication. The error HTML appears in IE11 for HTML and XHTML, verified during meeting.

chrisklus added a commit that referenced this issue Aug 19, 2020
chrisklus added a commit that referenced this issue Aug 19, 2020
chrisklus added a commit that referenced this issue Aug 19, 2020
@chrisklus
Copy link
Contributor

@zepumph and I discussed more today and figured out a pattern we both like. We decided on pulling the IE script back into its own file, converting it back to ES6, then transpiling separately when adding it to the initialization script during a build. This required adding an option to transpile.js for supporting IE, but was simpler than what I was talking about in #954 (comment).

This is working nicely on my machine and the output is appearing as expected. Marking as blocks-publication since this hasn't been retested on IE yet and assigning to @zepumph for review.

@chrisklus chrisklus assigned zepumph and unassigned chrisklus Aug 19, 2020
@zepumph
Copy link
Member Author

zepumph commented Aug 24, 2020

I need to get to this by the end of today!

zepumph added a commit that referenced this issue Aug 25, 2020
@zepumph
Copy link
Member Author

zepumph commented Aug 25, 2020

Review:

  • I understand why you put this under scripts/, but I don't feel like it belongs there too much. How about instead put it next to load-unbuild-strings.js and initialize-globals.js which are similar scripts.
  • Tested local build and XHTML in IE. The dialog showed up great!

Unblocking and over to @chrisklus for the last file move.

@zepumph
Copy link
Member Author

zepumph commented Sep 2, 2020

I did the last few bits. Closing

@zepumph zepumph closed this as completed Sep 2, 2020
samreid added a commit to phetsims/joist that referenced this issue Apr 16, 2022
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

9 participants