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

Add a hasThumbnail pseudo-attribute to the wp:query block #111

Closed
wants to merge 1 commit into from

Conversation

tellyworth
Copy link
Contributor

This lets us use wp:query {"query":{"hasThumbnail":true}} to query only for posts with a featured image/thumbnail.

I'm in two minds about the method of doing this; what do you think?

This lets us use `wp:query {"query":{"hasThumbnail":true}}` to query only for posts with a featured image/thumbnail.

I'm in two minds about the method of doing this; what do you think?
@tellyworth tellyworth requested review from ryelle and dd32 December 8, 2021 05:36
@dd32
Copy link
Member

dd32 commented Dec 8, 2021

I'm in two minds about the method of doing this; what do you think?

It's a hacky approach, but it seems like it'd work.. although I'm hesitant about any flow on effects of the WP_Query being detected as a search instance in the future.

Some upstream issues related: WordPress/gutenberg#31390 WordPress/gutenberg#24934

I guess a better question is, why? If it's just to ensure that the people-of-wordpress section only ever has items with thumbnails, I would be tempted to put that back on the onus of the post writer rather than ensuring that it's handled through code.. but since the code is here and works, why not. It's not going to conflict with any upstream changes AFAICT?

@tellyworth
Copy link
Contributor Author

Yeah potential side effects was my main concern. There might be a better side-channel to use than the search parameter but I can't think of one offhand.

As for why: There is no text design for the PoW block. Putting the onus on post authors creates one more way for things to go wrong, we might as well make it easy on them.

Also, part of the point of doing this as a block theme is to turn up shortcomings in core's support for block themes. Hopefully an upstream solution along the lines of hasThumbnail will turn up, in which case we can switch to that. Even if we don't use it this PR should illustrate the gap.

@ryelle
Copy link
Contributor

ryelle commented Dec 8, 2021

I also think this responsibility should be on the site authors - it doesn't break the rest of the site if they forget, it just looks awkward. IMO it would be more confusing if they added a person-in-wordpress and they don't show up, at least showing up without an image identifies the issue.

@iandunn
Copy link
Member

iandunn commented Dec 13, 2021

I agree with Kelly. We can iterate on the design so that it looks better when there's no image. IMO designs should embrace the nature of the web, rather than trying to force a "perfect" print mentality.

@iandunn
Copy link
Member

iandunn commented Dec 13, 2021

To encourage admins to upload photos, we could add an admin notice to alert them when they select the "people of wp" category and haven't set a featured image.

@iandunn
Copy link
Member

iandunn commented Dec 13, 2021

Another option would be a fallback image, like Gravatar's mystery person, except something more artistic.

We might need WordPress/gutenberg#32939 for that, though.

@tellyworth tellyworth marked this pull request as draft December 22, 2021 06:12
@shaunandrews
Copy link
Collaborator

shaunandrews commented Jan 12, 2022

Enforcing the featured image with code can help avoid some awkward displays, but it probably needs to be done alongside Ian's suggestion of some sort of messaging in the editor — otherwise people will publish and wonder why the post isn't showing.

That said, I think a fallback is probably the best solution; We could probably do it in the CSS with a background image if there is no featured image.

@shaunandrews
Copy link
Collaborator

@beafialho's suggestion was to use something similar to the Community category page and display text — maybe the person's name and the post date. Here's a quick mockup I made of how it could look:

image

@ryelle
Copy link
Contributor

ryelle commented Jan 28, 2022

Closing this PR, since the design already looks like the suggestion above 👍🏻

Screen Shot 2022-01-28 at 5 06 03 PM

@ryelle ryelle closed this Jan 28, 2022
@ryelle ryelle deleted the try/add-hasthumbnail-param branch June 1, 2022 16:50
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.

5 participants