Customizer Menus: link items should have view links also

[designsimply]

Moving this over from ​https://github.com/voldemortensen/menu-customizer/issues/104

From @celloexpressions:

We have handy "original" links for items that represent post or taxonomy term objects. We should add similar functionality for custom links and for bonus points, make it open in the Customizer preview if it's a previewable URL or otherwise open it in a new tab. Currently your best workflows are to copy/paste the URL or click on the link in the preview (which wouldn't work if it is an external link).

From @westonruter:

Note that original links for external custom links won't work in the Customizer either, since the Customizer preview will reject previewing any URL that is not under the site.

Semi-related: #32681

[celloexpressions]

Allow custom links to be previewed, or opened in a new tab if they aren't previewable.

[celloexpressions]

This is pretty straightforward - we can just let the link open normally (in a new tab, out of necessity) if it isn't previewable. The only thing I'm having trouble with in the patch is that ideally we could get back the result of the attempt to preview this URL here and use it as a conditional for calling e.preventDefault(). Not sure what the best way to do this is, anyone have thoughts?

Should also probably add live-updating of the URL here when the URL field is changed (not that it would be frequently). Having a preview link is actually pretty important - this functionality is typically present in other places where you add a link by URL so that you can check it before publishing. We can't assume that users know how to open links in new tabs themselves, or that they know they can do that in the Customizer preview if a link isn't working.

[westonruter]

Here's the logic for how the Customizer determines whether or not a URL can be previewed: ​https://github.com/xwp/wordpress-develop/blob/322b6cc54fcdcd078fe8fefd6e66ebe67dc7de16/src/wp-admin/js/customize-controls.js#L2507-L2543

So what you could do is something like this:

var originalLink = navMenuItemControl.container.find( '.original-link' );

originalLink.attr( 'target', '_blank' );
originalLink.on( 'click', function( e ) {
        var previousPreviewedUrl = wp.customize.previewer.previewUrl.get();
        if ( previousPreviewedUrl === this.href ) {
                // URL is already being previewed, so do nothing.
                e.preventDefault();
                return;
        }
        wp.customize.previewer.previewUrl.set( this.href );
        if ( previousPreviewedUrl !== wp.customize.previewer.previewUrl.get() ) {
                /*
                 * URL can successfully be previewed (and will now be), 
                 * so don't let URL open in new window.
                 */
                e.preventDefault();
                return;
        }
        /*
         * At this point, since the URL in the link is not the same as the currently-previewed
         * URL, and since the setter for wp.customize.previewer.previewUrl rejected the link 
         * URL as not being previewable, we let the browser do the default action and open 
         * the URL in a new window.
         */
} );

// Update the link whenever the menu item's URL changes.
navMenuItemControl.setting.bind( function ( newNavMenuItem, oldNavMenuItem ) {
        var previousPreviewUrl;
        if ( navMenuItem && newNavMenuItem.url !== oldNavMenuItem.url ) {
                originalLink.attr( 'href', navMenuItem.url );
        }
} );

[celloexpressions]

Implement @westonruter's suggestions.

[celloexpressions]

32711.2.diff​ implements westonruter's suggestions and should fix this ticket.

[celloexpressions]

This should be ready for commit.

[ocean90]

Having some links opening in a new tab and some in the preview feels a bit odd. Using "Original" and "View" for more or less the same functionality doesn't make it easier to understand this behaviour.

[celloexpressions]

Agree it's not ideal, but it's the best option here. Currently we don't provide any path to viewing link URLs, which is something users would want to do to check that it's typed properly, remind themselves of where it goes, etc. We really need to avoid direct links in the Customizer or other places where the link will navigate you away from the customization flow and your unsaved changes. Opening previable links in the Customizer preview is an awesome enhancement with ideal flow. For other links, we can still provide the most reasonable user flow - opening the link, but not navigating the page away from the Customizer - which is a bit disruptive but allows them to easily get back to the flow they were in.

In terms of view vs. original, for the original links the link text is the original title of the object that the menu item represents (this is taken from the admin screen handling). But "original" doesn't make sense for manual links that aren't tied to WordPress objects, so those get view.

On either of those issues, we shouldn't let these hold back this basic missing functionality in 4.3, so if you feel strongly, feel free to change them, but I disagree. Also, in the future (and this is also my fault for not pushing sooner), let's not let a ticket with a seemingly-finalized patch sit for 10 days without activity until we're cutting it super tight on beta.

[westonruter]

I just found a bug with the Original link: it is not populated for newly-added menu items.

[samuelsidler]

Replying to westonruter:

I just found a bug with the Original link: it is not populated for newly-added menu items.

Can you file a new ticket for that? (See also #32814.)

Just to recap this ticket:

Currently, when you add a "Page" to a menu, it includes an "Original" link that lets you view said page. This is true in nav-menus.php and in the Customizer. In the case of the Customizer, the "Original" link opens in the live-preview view.

However, for Custom Links, we don't currently show an "Original" link. This ticket proposes that we add an Original link if the content is on-site, by adding a check to see where the content is, and if it's not add a "View" link that opens in a new tab/window.

Per the discussion today (see Slack link above), this is wontfix. If a user adds a Custom Link, there's no expectation of being able to click to see their content. They can always copy/paste the URL. While non-ideal, it works and it's functional. Additionally, there's nothing preventing them from clicking the link in the live preview of the menu and navigating to the on-site link.

[westonruter]

Replying to samuelsidler:

Replying to westonruter:

I just found a bug with the Original link: it is not populated for newly-added menu items.

Can you file a new ticket for that? (See also #32814.)

Done. See #32858.