Skip to content
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(lib): handle callbackId in contextRequest #261

Closed

Conversation

demetriusfeijoo
Copy link
Contributor

@demetriusfeijoo demetriusfeijoo commented Aug 23, 2023

What?

Modify the requestContext action to return a Promise in place of returning nothing.

Why?

JIRA: EXT-1920

The corresponding pull request in Storyfront is a prerequisite: https://github.com/storyblok/storyfront/pull/4368

This PR has the same purpose (and approach) as this one but is now for the getContext action (not selectAsset)

Usage implementation:
image

Result:
image

How to test? (optional)

This function can be tested using our Sandbox application as described in or [CONTRIBUTING.md →]

@vercel
Copy link

vercel bot commented Aug 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plugin-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2023 2:33pm

@@ -2,7 +2,6 @@ import { createPluginActions } from './createPluginActions'
import {
AssetModalChangeMessage,
GetContextMessage,
HeightChangeMessage,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import was not being used

@eunjae-lee
Copy link
Contributor

I read it briefly, and it looks good!

Any doubt or question?

@demetriusfeijoo
Copy link
Contributor Author

Thanks @eunjae-lee.

I was more concerned about being in the right direction with this task @eunjae-lee.
No specific question for now.

@demetriusfeijoo demetriusfeijoo changed the title feat(lib): Change contextRequest() to return a Promise feat(lib): handle callbackId in contextRequest Aug 24, 2023
@demetriusfeijoo demetriusfeijoo changed the title feat(lib): handle callbackId in contextRequest feat(lib): handle callbackId in contextRequest Aug 24, 2023
@demetriusfeijoo demetriusfeijoo marked this pull request as ready for review August 24, 2023 00:22
@eunjae-lee
Copy link
Contributor

Hi Demetrius, Let's pause this task.

Johannes is bringing a big change around this: #260

We need more work on the Storyfront side to enable this change on the SDK side, but I'm very happy if we get this. Sorry that this PR probably is going to be closed (because it conflicts with #260), but I hope it was a good practice for you. My PR #253 also will be overridden by this, but we're moving very fast at this point, and it was a bit expected.

@demetriusfeijoo
Copy link
Contributor Author

Hey @eunjae-lee, pausing/closing this task will not be a problem.

This task helped me understand more about how the library works, so enough reward I'd say.

It was really cool working on it. 🚀

@eunjae-lee
Copy link
Contributor

closing this PR in favor of #260

@eunjae-lee eunjae-lee closed this Aug 30, 2023
@eunjae-lee eunjae-lee deleted the EXT-1920-library-update-request-context-method branch August 30, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants