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

Best way to support opening link in new tab #1768

Open
oliwoodsuk opened this issue Dec 21, 2023 · 3 comments
Open

Best way to support opening link in new tab #1768

oliwoodsuk opened this issue Dec 21, 2023 · 3 comments

Comments

@oliwoodsuk
Copy link

oliwoodsuk commented Dec 21, 2023

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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

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

4 participants