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

refactor(sandbox): remove container references #366

Conversation

demetriusfeijoo
Copy link
Contributor

@demetriusfeijoo demetriusfeijoo commented Mar 1, 2024

What?

  • It changes from packages/container to packages/sandbox
  • It renames the project name from container to sandbox
  • It adjusts the scripts located within the package.json to consider the new project's name
  • It replaces the term container with sandbox in the CONTRIBUTING.md file

Why?

JIRA: EXT-1470

The term container is too open, and not widely adopted by us when referring to the Sandbox. Replacing it will drive us to have a "ubiquitous language" which will also lead to a better DX.

PS: The Vercel action is not running yet since it wasn't modified to consider the new path (packages/sandbox) and also the new project name in its settings.

PS2: The Sandbox can run locally, but it isn't loading any plugin due to this issue. So, I advise you to check into this branch and perform the tests there since this one is on top of this.

Copy link

vercel bot commented Mar 1, 2024

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 Mar 7, 2024 0:34am

@demetriusfeijoo demetriusfeijoo force-pushed the EXT-1470-replace-some-occurrences-of-container-with-sandbox branch from be79526 to 041d57b Compare March 1, 2024 22:11
Copy link
Contributor

@BibiSebi BibiSebi left a comment

Choose a reason for hiding this comment

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

Thank you @demetriusfeijoo. Looking good :)

Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

This looks good to me!

I think, by this PR, it is clearer on the terms. Now that this plugin-sandbox.storyblok.com app will be called Sandbox all over the code, but now we can redefine the term "container".

, which means..

Container is a component that includes a field plugin within an iframe.

  • FieldTypeCustom (storyfront) is a container
  • We also have a container in the @storyblok/field-plugin/test package
  • We have a container inside this sandbox package.

So in this context, there is a container running within the sandbox environment (as opposed to the production, storyfront environment).

Then it'll be clearer and easier to extract the container part as a common code in the near future.

@demetriusfeijoo
Copy link
Contributor Author

Thanks for your reviews 💯 and I agree with the definition you brought, @eunjae-lee. 🙌

We could have a container connector that would be used for all the cases? 🤔 I mean, one library that should be able to be added to any project that wants to become a plugin container or to have its capacity.

It could be used for the Visual Editor, Plugin Editor, Sandbox, @storyblok/field-plugin/test, or any other.

@eunjae-lee
Copy link
Contributor

Thanks for your reviews 💯 and I agree with the definition you brought, @eunjae-lee. 🙌

We could have a container connector that would be used for all the cases? 🤔 I mean, one library that should be able to be added to any project that wants to become a plugin container or to have its capacity.

It could be used for the Visual Editor, Plugin Editor, Sandbox, @storyblok/field-plugin/test, or any other.

yeah I guess it would look something like createContainer(params) and can be used both in @storyblok/field-plugin/test and this Sandbox project.

Base automatically changed from EXT-2240-update-sandbox to main March 7, 2024 10:56
…replace-some-occurrences-of-container-with-sandbox

# Conflicts:
#	packages/sandbox/src/components/TranslatableCheckbox.tsx
#	yarn.lock
@demetriusfeijoo demetriusfeijoo merged commit 5f82f88 into main Mar 7, 2024
20 checks passed
@demetriusfeijoo demetriusfeijoo deleted the EXT-1470-replace-some-occurrences-of-container-with-sandbox branch March 7, 2024 12:48
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.

3 participants