-
Notifications
You must be signed in to change notification settings - Fork 435
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: adds support for Create-Studio integration #7635
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Oct 24, 2024 8:07 PM (UTC) ❌ Failed Tests (1) -- expand for details
|
⚡️ Editor Performance ReportUpdated Thu, 24 Oct 2024 20:21:04 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
config, | ||
context: {...context, ...partialContext}, | ||
initialValue: initialDocumentActions, | ||
propertyName: 'document.actions', | ||
reducer: documentActionsReducer, | ||
}), | ||
}) | ||
return getStartInCreateSortedActions(actions) |
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.
This is controversial, but necessary: It forces "Start in Create" action to be first in the actions list, when present.
This again results in "Start in Create" being the primary action, and thus shown in the document footer, outside the action-overflow menu.
If a plugin or studio config filters out the action, this sort does nothing.
Alternative implementation suggestions welcome.
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.
Ultimately a product question, but this happens only if 1) create integration is enabled in studio config and 2) only for new documents, right? If that's the case it seems fine to me
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.
(nvm, I see you called that out already in the PR description)
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.
My only thought as someone that doesn't know this part of the code super well is that some folks might wonder why, when trying to set a primary action themselves, they can't do it because the link always goes first.
I will say that it might be that you all have set up an exception for this and the use case, in itself, is an edge case that is out of scope for now.
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.
This is entirely intentional, since Product wants to faceplant this feature 🤷
Devs have two options:
- disable start in create
- OR filter out the action using actions: prev => {}.
Either approach will have the same result: No start in create button.
If you know how to do this via the config api I'm all ears though! I would love for this to end up first, but be moveable into the menu.
In a perfect world the plugin api had more expressive power, where we could set priority on actions or something, which would allow overriding when you really want to.
The alternative to this is to do something more drastic like I had to do for "Open in Sanity Create" and "unlink", ie a completly different codepath in structure. That, imo, is even worse of a smell.
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.
No no that does sound worse.
I unfortunately don't know how to do it off the top of my head since I don't know that part of the code that well. I'm assuming there is a way of doing: "if there is a custom primary action, then apply that first, otherwise do the open in sanity". 🤔
However, this is non-blocking because, like you mentioned, folks can disable it if they want or they can filter it out
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.
Just doing some final tests, and there is actually a (hacky) way to move the action into the menu.
The gist is: on the root defineConfig
, intercept the "Start in Sanity Create" action, and remove the action prop! 🙈
document: {
actions: (prev) => {
return prev.map((a) => {
if (a.action !== ('startInCreate' as any)) {
return a
}
//returs a new function that does not have a action prop
return (props) => {
return a(props)
}
})
},
},
Yup, its pretty dumb, but it can be done ;)
return { | ||
...beta?.create, | ||
startInCreateEnabled: !!beta?.create?.startInCreateEnabled, | ||
components: { |
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.
Notice how we provide structure components from core via context here.
Downside: introduces indirection
Upside: keeps most Create integration code in a directory structure in core
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.
I think my only concern about this is what I have commented previously about the banners.
Technically you could also export these directly from the sanity package (and then use them wherever you need to like the structure). Is there a reason why that wasn't considered vs this approach? 🤔
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.
I got to admit , I did not consider that approach. I've tried to limit what gets exported, as not to pollute the global `sanity´ api, was the thought. Therefore, only the props are exported, ie the interface. The implementations are internal.
I'd be happy to export the components, and ditch the context approach if you prefer.
To me, it just seems non-idiomatic to keep implementations without an interface in one package and use directly in another. I realize now that the way I'm going about this might break established sanity
conventions though, so I'm happy to take your lead on this.
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.
I don't think there is a conventional way per se of doing things; think of it more as a "mindless way that we've been going about it" than anything intentional :)
I just wanted to double check. I don't have a strong opinion about the way of exporting in particular and I'd be ok with this approach :)
import {createStartInCreateAction} from './start-in-create/StartInCreateAction' | ||
import {createAppIdCache} from './studio-app/appIdCache' | ||
|
||
export const createIntegration = definePlugin(() => { |
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.
Not everything needed for the Create integration can be expressed via the plugin API.
As much as possible has been put in here though. If anything else in this PR can be put here via existing apis, please let me know.
export const START_IN_CREATE_ACTION_NAME = | ||
'startInCreate' as unknown as DocumentActionComponent['action'] | ||
|
||
export function createStartInCreateAction(appIdCache: AppIdCache): DocumentActionComponent { |
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.
Yes, yes. Create is such a great app name :/
} | ||
export type AppIdFetcher = (projectId: string) => Promise<string | undefined> | ||
|
||
export function createAppIdCache(): AppIdCache { |
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.
Unclear to me if there are existing caching code available in the sanity package that I should rather be using.
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.
I don't think there is 🤔
@@ -533,7 +535,8 @@ export const DocumentPaneProvider = memo((props: DocumentPaneProviderProps) => { | |||
isLocked || | |||
isDeleting || | |||
isDeleted || | |||
isLiveEditAndDraft | |||
isLiveEditAndDraft || | |||
isCreateLinked |
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.
Not clear from the diff here, but this sets the documentPane readOnly
property.
Note: Not all codepaths respect document path readOnly
state. For instance, Paste document
can mutate a document pane in readOnly
mode. I contend that this is the right place to put this readOnly
code for Create though; code that does not respect it should be corrected elsewhere.
@@ -85,6 +88,9 @@ export function DocumentLayout() { | |||
const zOffsets = useZIndex() | |||
const previewUrl = usePreviewUrl(value) | |||
|
|||
const createLinkMetadata = getCreateLinkMetadata(value) | |||
const CreateLinkedBanner = useSanityCreateConfig().components?.documentLinkedBanner |
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.
This is done to avoid having to put component code belonging to the Create integration in the structure package.
const {editState, timelineStore, onChange: onDocumentChange} = useDocumentPane() | ||
const {title} = useDocumentTitle() | ||
|
||
const CreateLinkedActions = useSanityCreateConfig().components?.documentLinkedActions |
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.
This is done to avoid having to put component code belonging to the Create integration in the structure package.
@RitaDias I'm happy to report that |
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.
Great work :)
* next: feat: adds support for Create-Studio integration (#7635) fix(deps): Update dev-non-major (#7671) fix(deps): update dependency @sanity/insert-menu to v1.0.10 (#7668) fix(deps): update dependency @sanity/presentation to v1.17.6 (#7669) v3.62.2 fix(deps): Update dev-non-major (#7663) fix(deps): update dependency @sanity/presentation to v1.17.4 (#7662) fix(deps): Update dev-non-major (#7661) v3.62.1 feat(cli): add warning and docs for react-19 and Next.Js combined (#7660)
Description
This PR introduces the Studio side of the upcoming Create <-> Studio integration.
It allows users start their content editing journey in Create for new documents, by linking a Create document to a Studio document.
While linked, Create will sync content into the Studio document in real-time. Create linked Studio documents are read-only. Studio users can unlink the document at any point, to continue editing in the Studio. At this point Create will no longer sync content into the Studio document.
Update: Now sporting a beauty pass by @robinpyon 🙇
Update:
beta.sanity.startInCreateEnabled
will default tofalse
. Ie, the "Start in Sanity Create" button will be OPT IN for now. Documents that get linked from the Create side will still be put into read-only in the Studio. (this is also a form of opt-in tbh).What to review
Read through this full PR text before jumping into the code.
Changes affect:
change to userStore needs extra attention(This is no longer relevant, see below)There might be some controversial changes in here. I have made PR comments everywhere I though there might be some eyebrows raised, but there might be more. I'm very much open to course corrections.
Feature details
For Sanity folks: first see video of the full Studio-Create flow here.
In Studio deployments with an exposed
create-manifest.json
, new, prestine documents will have a "Start in Sanity Create" button in the document pane footer. See previous manifest PR for context.Clicking it will opens a confirmation dialog:
Clicking "Learn more" will open a "How to Create<->Studio" article. (Link url pending)
Clicking "Continue" will open a new tab which kicks you off to Create which will:
A studio document is considered "Create linked" whilst it has
_create.ejected === false
.While Create setting up the link (which entails setting the
_create
property), the following dialog will be shown after clicking "Continue":(displaying open state of the toggle)
Once the link is established, the document will be
readOnly
and the pane footer will be in a special "Create linked" mode:Clicking the info icon will open a popover:
Clicking anywhere outside (or pressing ESC) closes the popover.
Clicking "Edit in Sanity Create" will open up the Create document that points to this Studio document in a new tab.
Clicking "Unlink" will show the following dialog:
"Unlink now" will unset the
_create
property, thereby severing the link and making the document a regular Studio document. It will no longer bereadOnly
.High level implementation details
Code organization
I tried to:
For 1: this feature has some requirements that were not 100% plugin-supported, and required changes in core and structure:
For 2: I had to introduce a bit of indirection to make it possible to render stuff in structure.
SanityCreateConfigContext
contains two component implementations used by structure, provided from core.These are the components rendered when a document is Create linked. (Read only banner & Unlink actions).
For 3: I have added a BaseSchemaTypeOptions type. I always regretted not adding that for the initial v3 launch, since it allows plugins to add generic type-options much more easily. For instance, it could simplify the AI Assist type extensions quite a bit.
SanityCreateOptions
I have added
sanityCreate
options directly toBaseSchemaTypeOptions
. If it feels controversial to have them there, I can move them into a module declaration extension in core, but I feel that is only adding indirection with no upside.The idea is that
SanityCreateOptions
will be the DSL with which devs tailor their schema in Create (via the manifest file).Atm it supports
exclude
andpurpose
exclude
removes a type or field from appearing in Create. Excluded document types will not have a "Start in Sanity Create" button.purpose
supersedesdescription
, and will be used as metadata when describing the schema to an LLMWe need different options from AI Assist (which also has this exclude option), as Create has different needs than AI Assist.
Start in Create
We show the "Start in Sanity Create" button when:
beta.create.startInCreateEnabled: true
. Atm,startInCreateEnabled
defaults to false. This means studios will have to OPT IN._createdAt
)create-manifest-json
on a known url. It will only visit hosts listed under Studios in manage.documentOptions.sanityCreate.exclude !== true
Create link
A Studio document is considered linked to Create when it contains a
_create
metadata field.Specifically we check for
_create.ejected === false
. The idea is that we can keep the Create metadata around if we want to, but in this implementation we unset the full_create
field when unlinking.This ensures that this metadata does not end up in published documents.
If we want to do "soft unlinks" to keep a trace back to Create in the future (possibly opt-in via config), we can do that with with
_create.ejected: true
.Create link readOnly mode will always be enabled, regardless of what
beta.create.startInCreateEnabled
is.Getting the global user idEdit: We found issues with this approach and will remove the "global-id-in-url" approach.
As it turns out the user can log in with a different user in Create, resulting in the link-creation failing. Instead Create will handle all this, so we no longer need global user id in Studio.
To build the "Start in Sanity Create" url, we need the global user id.For current user, this is currently not available – but it IS for all other userIds. I propose a change to that, by no longer pre-priming the userStore with currentUser (at the cost of one more network request).See PR code comment for change to userStore.App-id cache
To build "Start in Sanity Create" url, we need the appId (deployed studio id) from
<project>/user-applications
.It is ok to cache this info for the duration of the Studio lifetime (ie, until browser refresh).
I dont know if there are existing caching mechanisms in core I could use instead of rollinga bespoke fetch-cache for this.
Telemetry
I've added telemetry for the following actions:
Open questions
Known caveats
Create does not respect
initialValues
, so these will be nuked when Create starts syncing.We might support that when we implement "Continue in Create from here" or something. But that is for another day.
Testing
For Sanity folks: follow the instructions here before starting .
Locally this integration can be tested by running the new test studio:
dev/test-create-integration-studio
. It hasfallbackOrigin
set, and will therefor have "Start in Sanity Create" on localhost.The integration can also be tested on https://create-integration-test.sanity.studio – it has been deployed from this branch.
Notes for release
The Create <-> Studio integration needs a full documentation article. There are a lot of moving parts.
Since
beta.sanity.startInCreateEnabled
will default tofalse
. Ie, the "Start in Sanity Create" button will be OPT IN, we can stealth launch this feature.Documents that get linked from the Create side will still be put into read-only in the Studio. If this happens, the editor has access to Create mapping and have probably read whatever Create-Studio documentation we have put out.
For everyone else, this PR effectively is a no-op.
TL;DR: We dont need release notes when this goes out with core. We will talk about Create-Studio at the next event, and then we can do something, as needed.