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

Update embedded app files #143

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Update embedded app files #143

merged 1 commit into from
Aug 20, 2024

Conversation

paulomarg
Copy link
Contributor

WHY are these changes introduced?

There were some files left over in the template that were either unused or outdated.

WHAT is this pull request doing?

  • Setting up the template so it can use Stimulus
  • Updating the frontend template reference
  • Using the latest App Bridge for non-React content

@paulomarg paulomarg requested a review from a team as a code owner August 14, 2024 15:37
@@ -13,27 +13,16 @@
<%= javascript_include_tag 'application', "data-turbolinks-track" => true %>
<% end %>
<%= csrf_meta_tags %>

<%= javascript_include_tag 'https://cdn.shopify.com/shopifycloud/app-bridge.js', data: {
'api-key': ShopifyApp.configuration.api_key,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be api-key or does it need to be shopify-api-key ?
https://shopify.dev/docs/api/app-bridge-library#getting-started

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! When passing in the key in the dataset, data-api-key works, that's what we use in the remix template as well.

I've tested this in a real app and it was able to make authenticated fetch requests, so app bridge was loading properly.

@paulomarg paulomarg merged commit 5e81bba into main Aug 20, 2024
7 checks passed
@paulomarg paulomarg deleted the update_non_react_files branch August 20, 2024 14:42
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