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

Allow configuring additional url schemes #287

Conversation

lewis-smith
Copy link

For mobile apps it's sometimes desirable to shorten deep links.

@ash-jc-allen
Copy link
Owner

Hey @lewis-smith, I love this idea!

I'm wondering if it'd be better using a short-url.allowable-url-schemes config value instead? That way, in the package's config file, we could have http:// and https:// already set in there and any more could be added to that list in the dev's overridden config file. I guess this could be handy because the dev could also remove the http:// prefix if they only want to allow https:// URLs too. Do you have any thoughts on this? 🙂

@lewis-smith
Copy link
Author

@ash-jc-allen I'm glad you like the idea :)

From my perspective, in the short term I just want to create links to my app and so as long as I can do that I'm happy to go with whatever approach your recommend :)

I suppose the risk with your approach is that if you move http and https into config it would break existing installs? Maybe just http goes to config?

@ash-jc-allen
Copy link
Owner

@lewis-smith Sweet! I should have a bit of free time later today, so I'll make a couple of small updates to this PR and then get it merged 😄

@ash-jc-allen ash-jc-allen changed the base branch from master to allowed-url-schemes July 2, 2024 17:20
@ash-jc-allen ash-jc-allen merged commit 7038c04 into ash-jc-allen:allowed-url-schemes Jul 2, 2024
10 of 11 checks passed
@ash-jc-allen
Copy link
Owner

Hey @lewis-smith! Huge thanks again for this idea. I've made a couple of small changes and got it merged in. I'll get it released shortly 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants