Add a query parameter allowLinks=false

[samreid]

From today's discussion, we would like to add a query parameter allowLinks=false, so that links can't be followed out of the sim.

[samreid]

I added this support above. @zepumph has more notes from this discussion. @jessegreenberg I noticed

      // a11y
      tagName: 'div'

In the other VBox, should it be added to the sub-VBox as well?

@zepumph can you please review?

[pixelzoom]

Yesterday we speculated that the About dialog was the only place where hyperlinks appear. That's not the case - Molecule Polarity has a link in the "Real Molecules" screen, see screenshot below. According to @ariel-phet and @kathy-phet, we don't need to do anything about this.

[zepumph]

should it be added to the sub-VBox as well?

I don't think so. In general proliferation of div elements in the PDOM is less than ideal.

I feel like this should also be implemented to support other common code links, even if they are not totally needed at this time, for completeness. Through my search I found two more spots:

UpdateNodes has a linked RichText than ends up in the AboutDialog when out of date. I propose that UpdateNodes.createOutOfDateAboutNode changes to:

    createOutOfDateAboutNode: function( options ) {
      const linkNode = phet.chipper.queryParameters.allowLinks ? new RichText( '<a href="{{url}}">' + updatesOutOfDateString + '</a>', {
        links: { url: updateCheck.updateURL }, // RichText must fill in URL for link
        font: updateTextFont
      } ) : new Node();
      return new HBox( merge( {
        spacing: 8,
        cursor: 'pointer',
        maxWidth: MAX_WIDTH,
        children: [
          new FontAwesomeNode( 'warning_sign', { fill: '#E87600', scale: 0.5 } ), // "safety orange", according to Wikipedia
          linkNode
        ],

        // a11y
        tagName: 'div'
      }, options ) );
    },

UpdateNodes turned me onto PHET_CORE/openPopup and made me wonder if we should wrap that functions around an allowLinks check.

What do you think about these improvements?

[zepumph]

Back to you @samreid, sorry I didn't do so earlier.

[samreid]

It seems like we should address this before making more releases.

[jessegreenberg]

Regarding #592 (comment)

I agree with @zepumph, it is only needed for the container of all dialog content.

[samreid]

At the previous dev meeting, we agreed I need to understand the recommendations in #592 (comment) better and coordinate with @zepumph on next steps.

[samreid]

With the proposed code, the about dialog looks like this:

I'll add the text, but make it non-linked and non-pointered.

[samreid]

I committed the revision above, here is a patch to facilitate testing:

Index: js/AboutDialog.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/AboutDialog.js	(revision f3b2bd1f971e34b965d30889b64d771b5adcd687)
+++ js/AboutDialog.js	(date 1579185559926)
@@ -71,7 +71,7 @@
       } ) );
     }
 
-    if ( updateCheck.areUpdatesChecked ) {
+    if ( updateCheck.areUpdatesChecked || true ) {
       const positionOptions = { left: 0, top: 0 };
       const checkingNode = UpdateNodes.createCheckingNode( positionOptions );
       const upToDateNode = UpdateNodes.createUpToDateNode( positionOptions );
@@ -83,6 +83,7 @@
 
       // @private {function(UpdateState)} - Listener that should be called whenever our update state changes (while we are displayed)
       this.updateVisibilityListener = function( state ) {
+        state = UpdateState.OUT_OF_DATE;
         checkingNode.visible = state === UpdateState.CHECKING;
         upToDateNode.visible = state === UpdateState.UP_TO_DATE;
         outOfDateNode.visible = state === UpdateState.OUT_OF_DATE;

@zepumph can you please review at your convenience?

[zepumph]

This is really nice! I especially like the ternary around setting links. That feels suave to me. No other thoughts here.

[samreid]

Thanks, it seems this issue is ready to close.