-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(tool-next): support App Bridge #85
feat(tool-next): support App Bridge #85
Conversation
…y-for-nextjs-starter-tool-plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @eunjae-lee
Sorry for the delay on this one 🙏
The changes look solid 💯 but I left some nit comments and questions just to be sure once this context is still complex to me.
I was also questioning about moving the pages/api/_app_bridge.ts
and pages/api/_oauth.ts
files to a subfolder, like pages/api/_auth/
. Wdyt?
By the way, missed a lot we don't have the Layers and Middlewares and in Nuxt here. 🤣 🤣
Yeah indeed we don't have Layers and Middlewares, and I don't like the outcome compared to our nuxt base 😭 Anyway, we could change the endpoints, but we should do it for all the other projects for consistency. Maybe we could do it in a separate PR if we want. I will exclude it for now here. |
Co-authored-by: Demetrius Feijóo <[email protected]>
Co-authored-by: Demetrius Feijóo <[email protected]>
…s-starter-tool-plugin' of github.com:storyblok/space-tool-plugins into SHAPE-7069-implement-app-bridge-functionality-for-nextjs-starter-tool-plugin
Co-authored-by: Demetrius Feijóo <[email protected]>
…s-starter-tool-plugin' of github.com:storyblok/space-tool-plugins into SHAPE-7069-implement-app-bridge-functionality-for-nextjs-starter-tool-plugin
Co-authored-by: Demetrius Feijóo <[email protected]>
Hey @eunjae-lee
Yeah, I totally agree. Let's do it in another PR and it's also not that relevant too. 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's gooo 🚀 🚀
Massive job, @eunjae-lee 💪
@@ -0,0 +1,9 @@ | |||
export const KEY_PREFIX = 'sb_ab'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question @eunjae-lee @demetriusfeijoo is this supposed to be sb_app
or sb_ab
? Is okay? What is ab
in this context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app.. bridge... 😂
{completed && ( | ||
<div> | ||
<UserInfo /> | ||
<Test /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be <Example />
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. I missed it! Let me make a PR.
What?
This PR adds App Bridge support to the Next.js Tool Starter. It's mostly copy & paste from space-plugins/nextjs-starter.
Why?
JIRA: SHAPE-7069
How to test? (optional)