-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 badge to title for posts & front pages #61718
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +195 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
</Link> | ||
) : ( | ||
decodeEntities( item.title?.rendered ) || | ||
__( '(no title)' ) | ||
<> |
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 an HStack
instead of a fragment here might allow better spacing (a small gap or could also allow to use space-between if needed)
c4e2af0
to
078d1e5
Compare
Given my time-constraints, I leave two suggestions for follow-ups (also happy if anyone wants to own this PR and do the tweaks here):
|
@oandregal mind if I push a couple small CSS tweaks |
Please, be welcome to do so! 🙇 |
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.
LGTM 👍
@@ -273,6 +273,16 @@ export default function PagePages() { | |||
[ totalItems, totalPages ] | |||
); | |||
|
|||
const { frontPageId, postsPageId } = useSelect( ( select ) => { |
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.
Potential follow-up: I think this one can potentially move to the "render" function instead (or should I say render component) (we should treat them as component in DataViews first though) which will make sure it only runs if the field is actually rendered.
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.
Title is always rendered in pages, so it would be the same..
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, but still :) conceptually it makes sense for this to be within the field.
Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Fixes #57763
What?
Adds a badge to indicate that a page is the
Front Page
or thePosts Page
.Why?
This is the target design we want:
How?
Update the
title
rendering function to check whether the item is one of the special pages. If so, add a badge.Testing Instructions