laravel-shopify icon indicating copy to clipboard operation
laravel-shopify copied to clipboard

Verify theme support

Open enmaboya opened this issue 2 years ago • 12 comments

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

enmaboya avatar Sep 04 '22 14:09 enmaboya

@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.

gnikyt avatar Sep 05 '22 15:09 gnikyt

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.

Kyon147 avatar Sep 06 '22 11:09 Kyon147

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

enmaboya avatar Sep 07 '22 07:09 enmaboya

@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.

gnikyt avatar Sep 08 '22 14:09 gnikyt

@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

Kyon147 avatar Sep 08 '22 14:09 Kyon147

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

gnikyt avatar Sep 08 '22 14:09 gnikyt

@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.

gnikyt avatar Sep 09 '22 14:09 gnikyt

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

I also fixed conflicts with the main branch

enmaboya avatar Sep 11 '22 13:09 enmaboya

Thanks! Will await Kyron147 review before we merge.

gnikyt avatar Sep 21 '22 12:09 gnikyt

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

Apologies work has been manic the last week or so!

Kyon147 avatar Sep 21 '22 12:09 Kyon147

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

gnikyt avatar Sep 21 '22 12:09 gnikyt

@Kyon147 I agree.

gnikyt avatar Oct 03 '22 16:10 gnikyt