Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.74k stars 685 forks source link

Best way to support opening link in new tab #1768

Open oliwoodsuk opened 7 months ago

oliwoodsuk commented 7 months ago

Overview

I spent a day or so yesterday trying to figure out what the best way to support the scenario where a user command-clicks a link, so it opens in a new tab from within an embedded app.

We were previously on v18.1.2 of this gem, and it seemed to be handled natively by the gem when the ShopifyApp::EnsureAuthenticatedLinks concern was included. Whilst in v21.8.0 of this gem, that concern adds support for deep links, it doesn't add support for opening pages in new tabs. However, it does work if a shop and host param is present on the request URL. If I'm remembering correctly from yesterday, it looked like ShopifyAPI::Auth was responsible for part of this behaviour.

So my question is, is there a way native to this gem that allows us to support opening links in new tabs? We're currently solving this by dynamically adding the params to <a> hrefs from stored .window attributes via JS. This seems a bit hacky, maybe there's some kind of header support that could do the same thing? or maybe there's just a setting somewhere I've overlooked.

Thanks for your help,

Oli


// Allows new tabs to work by adding a shop and host URL param. 
// i.e. the new page redirects to the shopify admin, then splash page, then the return_to location 

document.addEventListener('DOMContentLoaded', () => {
    document.addEventListener('click', (event) => {
        const element = event.target.closest('a');
        if (!element) return;

        if (shouldInterceptLink(element)) {
            const newUrl = urlWithShopifyParams(element.getAttribute('href'));

            element.href = newUrl;
        }
    });
});

function shouldInterceptLink(element) {
    return element.href && !element.href.startsWith('mailto:') && isInternalLink(element);
}

function urlWithShopifyParams(url) {
    const urlObj = new URL(url, window.location.origin);
    if (window.shopOrigin && !urlObj.searchParams.has('shop')) {
        urlObj.searchParams.set('shop', window.shopOrigin);
    }
    if (window.shop_host && !urlObj.searchParams.has('host')) {
        urlObj.searchParams.set('host', window.shop_host);
    }
    return urlObj.toString();
}

function isInternalLink(element) {
    const url = new URL(element.href);
    return url.origin === window.location.origin;
}

...bit of context We're running a Rails 7.1 app with Turbo. Below are our controller setups.

class AuthenticatedController < ApplicationController
  include ShopifyApp::EnsureAuthenticatedLinks
  include ShopifyApp::EnsureHasSession

...
class SplashPageController < ApplicationController
  skip_before_action :verify_authenticity_token
  protect_from_forgery with: :null_session

  include ShopifyApp::EmbeddedApp #sets layout as .embedded_app.html.erb
  include ShopifyApp::EnsureInstalled
  include ShopifyApp::ShopAccessScopesVerification

...
flavio-b commented 6 months ago

What we ended up doing is creating a helper that wraps paths or URLs with the right attributes. Something like this:

<%= link_to 'Example', with_params_for_new_tab(example_path) %>. And we used only in select links that users might be more inclined to open in new tabs, like top navigation, etc.

Another more intrusive alternative that catches all links, is to override your path and URL helpers in your authenticated controllers, to include the params by default.

But yeah, it would be nice to have a default helper for this in the gem.

oliwoodsuk commented 6 months ago

@flavio-b ok cool, so at least it's not just me missing something in the gem. Mm, I think the link_to approach is less dirty for sure. We'll probably stick with the JS approach for now though. The JS code above needed some tweaks for cases like anchor links and javascript:void.

matteodepalo commented 6 months ago

Hi @oliwoodsuk, thank you for opening this issue! This is a great suggestion, the team is going to review it and look into it.