Make WordPress Core

Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#31517 closed enhancement (fixed)

Customizer: show a notice after attempting to navigate to external links in live previews

Reported by: designsimply's profile designsimply Owned by: westonruter's profile westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:

Description (last modified by westonruter)

The Customizer only allows you to click internal, front-end links within a preview to navigate to URLs you can customize:
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/js/customize-controls.js#L2110

Clicking on an external link from the live preview pane fails silently. To help avoid confusion, we could display a notice to alert when someone clicks on an external link in a live preview:

You can only preview internal, front-end links in the customizer.

Here is a past description of how a customizer notice could work from @westonruter: https://core.trac.wordpress.org/ticket/30677#comment:7 (different use case, but the part about how the alert might work is relevant)

#29098 provides a general mechanism for sending back error codes/messages.

Change History (13)

#1 @westonruter
9 years ago

  • Milestone changed from Awaiting Review to Future Release

I implemented this actually as part of my transactions proposal: #30937.

As in my pull request:

It outputs the following CSS to the Customizer preview:

.customize-preview-not-allowed,
.customize-preview-not-allowed * {
    cursor: not-allowed !important;
}

And the following JS is added to the Customizer preview as well:

$( 'form[action], a[href]' ).each( function () {
        var url, el = $( this );
        url = el.prop( 'href' ) || el.prop( 'action' );
        if ( url && ! self.isAllowedUrl( url ) ) {
                el.addClass( 'customize-preview-not-allowed' );
                el.prop( 'title', api.settings.l10n.previewNotAllowed );
        }
});

This ensures the tooltip and the proper cursor are displayed.

And then when actually trying to click on a link (or submit a form), it prevents the default action:

 this.body.on( 'click.preview', 'a', function( event ) {
        var to = $( this ).prop( 'href' );

        // @todo Instead of preventDefault and bailing, should we instead show an AYS/confirm dialog?

        if ( ! self.isAllowedUrl( to ) ) {
                event.preventDefault();
        }
});

this.body.on( 'submit.preview', 'form', function( event ) {
        var form = $( this );
        if ( ! self.isAllowedUrl( this.action ) ) {
                event.preventDefault();
                return;
        }
        // ...
});

Currently in Core, the logic for isAllowedUrl is handled in the Customizer pane. The transactions patch moves this logic to the preview instead.

#2 @westonruter
8 years ago

  • Milestone changed from Future Release to 4.6

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


8 years ago

#4 @celloexpressions
8 years ago

The basic functionality is in the transactions patch. Let's revisit this ticket once that lands to make sure we handle it in an aaccessible, user-friendly way, and account for all of the cases listed in the above slack conversation.

#5 @westonruter
8 years ago

See also discussion regarding a preview heartbeat to catch scenarios where a user is navigated to another URL without the Customizer preview context: https://wordpress.slack.com/archives/core-customize/p1462222433000451

#6 @westonruter
8 years ago

  • Milestone changed from 4.6 to Future Release

Punting due to transactions being too large to finish patch and test in time for 4.6.

#7 @westonruter
7 years ago

  • Description modified (diff)
  • Keywords has-patch needs-testing added
  • Milestone changed from Future Release to 4.7
  • Owner set to westonruter
  • Status changed from new to accepted

This has been implemented in the patch for #30937.

When a link is not previewable it will get a cursor:not-allowed style and wp.a11y.speak() will be called to accessibly notify that it cannot be live previewed. Likewise, when attempting to submit a form that is not live-previewable (if it uses a POST method or has an action that points off of the site) there will be be the same cursor:not-allowed style added to the form and all of its inputs with a similar non-previewable message output via wp.a11y.speak().

Please test: grunt patch:https://github.com/xwp/wordpress-develop/pull/161

Last edited 7 years ago by westonruter (previous) (diff)

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 years ago

#9 @celloexpressions
7 years ago

Can we do anything here for touch devices? We probably need a notice after a link is clicked or a form is submitted, which would integrate with #35210.

#10 @westonruter
7 years ago

@celloexpressions There's nothing special for touch, no. As such, mobile is out of scope for this ticket and mobile users would have no change in behavior from what is existing in core. It is not clear how a notice could be displayed. A notice for mobile users should perhaps be split off into a separate ticket.

#11 @celloexpressions
7 years ago

Touch and mobile are not the same thing :) However, I'm okay with deferring mobile to another ticket. We need to make sure we consider that cursor: not-allowed is only accessible to mouse users, and wp.a11y.speak() is only accessible to screen reader users. We'll need a solution for touch and keyboard-only users as well, but those could come later if needed.

#12 @westonruter
7 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 38810:

Customize: Implement customized state persistence with changesets.

Includes infrastructure developed in the Customize Snapshots feature plugin.

See https://make.wordpress.org/core/2016/10/12/customize-changesets-technical-design-decisions/

Props westonruter, valendesigns, utkarshpatel, stubgo, lgedeon, ocean90, ryankienstra, mihai2u, dlh, aaroncampbell, jonathanbardo, jorbin.
See #28721.
See #31089.
Fixes #30937.
Fixes #31517.
Fixes #30028.
Fixes #23225.
Fixes #34142.
Fixes #36485.

#13 @westonruter
7 years ago

In 41338:

Customize: Do not show cursor: not-allowed on audio/video track titles within playlists in preview.

Props scott.deluzio, mitraval192, westonruter.
See #31517.
Fixes #41489.

Note: See TracTickets for help on using tickets.