search

gnikyt / laravel-shopify

A full-featured Laravel package for aiding in Shopify App development
MIT License
1.24k stars 374 forks source link

Verify theme support #1198

Closed enmaboya closed 8 months ago

enmaboya commented 2 years ago

As you know lately, Shopify does not allow applications that only use script tags.

To work with the Online Store 2.0 you need to use "Theme app extensions". But in this case you should not install script tags. If the store has not yet updated the theme and is using the Online Store 1.0, script tags must be installed.

These changes solve this problem

gnikyt commented 2 years ago

@enmaboya While I disagree with Shopify introducing this, it just adds bloat and grunt work for people simply trying to make apps.. thanks for the changes, tests are in place and seem to be functioning. I am fine with this so long as it does indeed still support both methods. May need to add some documentation as well for people in the wiki.

Kyon147 commented 2 years ago

Nice work on this @enmaboya, it is defo way too complicated than it needs to be (from Shopify)

It's worth mentioning that app embed blocks don't require script tags as they work with both v1 and v2 of themes.

The dual script is specifically for App Blocks https://shopify.dev/apps/online-store/theme-app-extensions/extensions-framework#app-blocks.

Might be worth putting that in the docs as well - as I agree with @osiset this is going to need some documentation as it's a pretty big uplight in functionality.

p.s code coverage seems to be failing though which needs looking at.

enmaboya commented 2 years ago

@osiset @Kyon147 I'll do the documentation and tests in the next few days

gnikyt commented 2 years ago

@enmaboya Additionally, if app devs are expected to now install code snippets in an automated way to help with app blocks, where is this part of the code, or does it exist? Just trying to see if I can wrap my head around the flow.

Kyon147 commented 2 years ago

@osiset the app blocks can be initiated using the Shopify CLI https://shopify.dev/apps/online-store/theme-app-extensions/getting-started#step-1-create-a-new-extension - so adding how to do this in a basic form inside the wiki would be helpful I think.

This just scaffolds a directory inside your project root and any "updates" to the extension are the also published using the CLI.

This is at least how I have used it recently when using an App Embed block but have not used the other type of block which requires this code @enmaboya has done but looking at the docs, the code for the extension works the same.

Here's a sample of the structure inside my directory after you run the CLI. image

gnikyt commented 2 years ago

@Kyon147 Makes sense, I did it with a node project before, just wasnt sure how it would function with the Laravel setup, neat!

gnikyt commented 2 years ago

@enmaboya I have added unit test for your theme helper service, resolved conflicts, and updated the documentation for it.

@Kyon147 I am current good with this too if you are. Tests are passing.

enmaboya commented 2 years ago

@osiset Cool! I couldn't find time for them.

I also fixed conflicts with the main branch

gnikyt commented 2 years ago

Thanks! Will await Kyron147 review before we merge.

Kyon147 commented 2 years ago

Hey @osiset I'll carve sometime this week to take a look.

Apologies work has been manic the last week or so!

gnikyt commented 2 years ago

@Kyon147 No rush, no need for apologies :) been a hectic couple years myself, everyones busy ha

gnikyt commented 2 years ago

@Kyon147 I agree.