-
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
DataViews: Add featured image field to the page list #55246
Conversation
Size Change: +338 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
Flaky tests detected in f8f8a93d76623cbe36607a6465a3da6fe0cf7d1a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6480475946
|
f8f8a93
to
69e0750
Compare
const sizesPerPriority = [ 'large', 'thumbnail' ]; | ||
const currentSize = | ||
size ?? | ||
sizesPerPriority.find( ( s ) => !! media?.media_details?.sizes[ s ] ); |
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'm picking nits, but xs.find( x => !! f( x ) ) is better expressed as xs.some( x => f( x ) ) Edit: I misread, it's just that the !!
is unnecessary.
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.
@mcsf I'm kind of lazy to open a PR just for these 😬
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.
Yeah, I know, no need to open one! Just an FYI, and then whoever of us is touching these files may or may not decide to change it. :)
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.
@mcsf some
will return a boolean value and not the size though.
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.
Oh yeah, I misread the intention here. So it's just that !!
is unnecessary, but find
is still the right idiom. Carry on. <homer-disappear.gif>
header: __( 'Featured Image' ), | ||
accessorFn: ( page ) => page.featured_media, | ||
cell: ( props ) => | ||
!! props.row.original.featured_media ? ( |
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.
Still picking nits with !!
: this cast-to-boolean inside the ternary condition is unnecessary. :)
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 like these when the props are not booleans 😆
Related #55083
What?
The grid data view layout requires items to have a media to render per item. So before starting to explore the grid layout, I wanted to add a "media" field to the table view. This small PR just adds the featured image field to the pages list data view in the site editor.
I wanted to ensure that this field is actually hidden initially, in the initial view config but it's not possible yet to control the visibility from the view config. I believe @ntsekouras is working on that in parallel.
Testing Instructions
1- Add some pages with featured images
2- Make sure the featured image column in visible properly in the table view