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

Fix issue where we get 'Unable to resolve image' if the user does not… #18

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

roblevintennis
Copy link
Contributor

@roblevintennis roblevintennis commented Oct 3, 2022

This fixes an issues I encountered first stab at getting up and running:

  • where I hadn't selected a post image on the Sanity CMS side for a particular blog post. I think we should probably be able to handle that as a valid use case hence my fix. It seems that null is working fine as the fallback there. Also, I noticed that everywhere else in the code we're using conditional rendering with {post.mainImage &&... so this feels pretty consistent with that ;-)
  • Missing Author name
  • default publishedDate and validation required

@roblevintennis
Copy link
Contributor Author

roblevintennis commented Oct 3, 2022

I noticed a similar thing for Published at--for example my vanilla Sanity CMS is allowing me to leave this blank:
image

And then you may get:
image


UPDATE: I think if we just make it required in the schema it'll at least, ahem, require it from the CMS:
https://www.sanity.io/docs/datetime-type#required()-317a51883c16

Added in 696ccd1

Tested:
image

… gets to client and user understands it is required.
@@ -41,6 +41,7 @@ export default {
name: 'publishedAt',
title: 'Published at',
type: 'datetime',
validation: Rule => Rule.required(),
Copy link
Contributor Author

@roblevintennis roblevintennis Oct 3, 2022

Choose a reason for hiding this comment

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

Based this off of the initialValue templates docs: https://www.sanity.io/docs/initial-value-templates#dec5ffddbcc2 which shows how to prefill such fields

@roblevintennis roblevintennis marked this pull request as draft October 4, 2022 01:06
@roblevintennis roblevintennis marked this pull request as ready for review October 4, 2022 01:16
@roblevintennis
Copy link
Contributor Author

roblevintennis commented Oct 4, 2022

e341fcb also fixes:
#14

When I played in the GROQ playground it seems that "it likes" the ellipsis to be at the beginning. These examples on their docs site do that though I didn't find it mentioned that it's a requirement but I guess so.

Note that we're getting some author data:

image

I took liberty to add the other author properties since it's now coming back.

@@ -1,7 +1,7 @@
import { useSanityClient } from "astro-sanity";

export async function getAllPosts() {
const query = `*[_type == 'post']{"categoryData": categories[]->{slug, title},author -> {name}, ...} | order(publishedAt desc)`;
const query = `*[_type == 'post']{..., "categoryData": categories[]->{slug, title}, author->{name, slug, image, bio}} | order(publishedAt desc)`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Besides moving the ... I remove extra spaces as I noted the examples on their docs don't have spaces and also you don't in the arrow for categories[]->{slug, title} for example so it overall seemed the right thing to do ;-)

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.

1 participant