-
Notifications
You must be signed in to change notification settings - Fork 1
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: add featured projects to registry home #49
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
export default { | ||
type: 'object', | ||
name: 'homePageProjectsSection', | ||
title: 'Home Page Projects Section', | ||
fields: [ | ||
{ | ||
name: 'titleCustomBody', | ||
type: 'titleCustomBody', | ||
title: 'Title and Body', | ||
validation: Rule => Rule.required(), | ||
}, | ||
{ | ||
title: 'Featured Projects', | ||
name: 'projects', | ||
type: 'array', | ||
of: [ | ||
{ | ||
type: 'reference', | ||
to: [{ type: 'project' }], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we don't necessarily want a reference to an existing project in Sanity because this means the editor will need to provide project image, location, etc. if the project doesn't already exist while we can easily get this from the marketplace app (contrary to the website at this point), so I guess just a list of strings could be sufficient. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a list of strings here would prevent editors from reusing previously used IDs. Searching a project by name is also a better experience in sanity than having to input an ID manually. By doing so we also capitalize on a single source of truth for a project that could be helpful elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah then if we only make name and id required, I agree that works better There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's done |
||
}, | ||
], | ||
}, | ||
], | ||
}; |
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 a side note that this is breaking and leads to the following error on https://dev.app.regen.network/ and all deploy previews that do not include your code from regen-network/regen-web#2191 until this is merged
so it might be best to make use of sanity graphql tagged endpoints: https://www.sanity.io/docs/graphql#e2e900be2233
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 wasn't aware we could create additional tags like that, will make more use of that in the future if needed.
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'll create an issue to add a note in the README about that