Change how we check if app is embedded #1458
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
We want to support unframed apps. Note that this only changes the redirect functionality within the app; there is no change to the frontend which is still using App Bridge v2.
WHAT is this pull request doing?
Using
appBridgeUtils.isShopifyEmbedded()
rather than manually checkingwindow.top
. isShopifyEmbedded is a more complex check that supports unframed.Also, upgrade to App Bridge 3.1.1 (from 2.0.12) as this is required, in the redirect functionality only.
Closes https://github.com/Shopify/first-party-library-planning/issues/366
Reviewer's guide to testing
Test environment:
3.1.2p20
7.0.3
2.3.16
v18.3.0
Gemfile
. Modify the entry toshopify_app
to point to this branch in your local environment:bundle install
.shopify app create rails
command - you may need to run any commands that didn't complete successfully. For example, in my case, I needed to runbin/rails importmap:install
, followed bybin/rails turbo:install stimulus:install
, andbin/rails generate shopify_app --new-shopify-cli-app
.app serve
commandapp/assets/javascripts/shopify_app/redirect.js
to see more output in browser console:Test new installation flow
Test the login page on shop already has the app installed
Test re-authentication. Ex: cookies is expired
shops
tableBrowsers
Perform the tests above for Chrome, Firefox and Safari. Note that you will likely need to disable any third-party cookie blocking that Firefox and/or Safari have enabled, as well as any "enhanced protection" settings (Firefox).
Checklist
Before submitting the PR, please consider if any of the following are needed:
CHANGELOG.md
if the changes would impact users - not visible to userUpdate- not visible to userREADME.md
, if appropriateUpdate any relevant pages in- not applicable/docs
, if necessary