search

Shopify / shopify-node-app

An example app that uses Polaris components and shopify-express
MIT License
328 stars 136 forks source link

Upgrade to Polaris 2.0 #120

Closed dompenn2010 closed 6 years ago

dompenn2010 commented 6 years ago

I wanted those DataTables baaaad.

Following directions from the Polaris 2.0 change log.

This only thing I ran into is that I had to change my Apps Whitelisted redirection URLs from:

https://topre_is.rubber.domes/auth/shopify/callback to: https://topre_is.rubber.domes/shopify/auth/callback

This threw me for a loop for a long time!

This resolves #117

Reminder: This is a major version bump. There are breaking changes. However, it made little sense for the seed app to not have the new awesome Polaris features!

dompenn2010 commented 6 years ago

@TheMallen I noticed you were pretty active here, please let me know what you think about the PR. (I wasn't able to assign a reviewer)

Either way I'll be using the changes for personal use :)

dompenn2010 commented 6 years ago

Meh, seems like there's been many more changes than this PR will cover.

The introduction of the Admin api and much more.

My apologies but #120 won't do the trick anymore.

dompenn2010 commented 6 years ago

I'll leave it here in hopes that it helps build interest in an eventual fix by Shopify.

devshahani commented 6 years ago

Hey @dompenn2010 the PR looks fine to me so far - what other changes are you concerned about in regards to this not working?

dompenn2010 commented 6 years ago

Hey @devshahani, thanks for touching base.

I don't recall everything I was thinking off the top of my head, but I believe I ran into #121 (which I'm unsure whether or not I resolved) as well as some issues that I believe were stemming from the revamp of the Order API (I believe it was).

Apologies, it's been a few weeks. I'll go ahead and update that package now, and take a look at the other stuff this weekend. Hopefully I'm just incorrect :)

devshahani commented 6 years ago

@dompenn2010 I tested your branch and it worked as expected. I do see there are a few issues that need to be addressed with this repo, but I don't think that should come in the way of upgrading to Polaris 2. Thanks for your contribution!

dompenn2010 commented 6 years ago

@devshahani Awesome, thanks for taking care of that!

Looking forward to further contributions.