-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
QuickEdit: Add Featured Image Control #64496
Changes from 45 commits
785085b
dd7e0a3
77f2217
433e45f
f890963
c32622e
9a39fe6
5419b0a
68ef300
cd923f8
94335a4
030a28d
baf7a82
4095dab
23d8695
ec715ed
ea29690
106af4d
cc46724
b11e923
c6dffd7
3c262d6
c053a9a
57faa8f
d2eeb0e
05f0d1a
c4db3b9
cd9fe2f
8356977
e27dec1
a187591
8815e5a
a461d85
fcc2e6e
577f836
a3449c4
2366429
1b84ac0
6ca01ff
8e612d4
c448c49
2b595c2
9cc6d80
01a57a8
7b1e40a
6592d36
c2994e0
b039310
42f90f4
ea8e941
e77b93b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,6 +24,7 @@ import SingleSelectionCheckbox from '../../components/dataviews-selection-checkb | |||||||||||
import { useHasAPossibleBulkAction } from '../../components/dataviews-bulk-actions'; | ||||||||||||
import type { Action, NormalizedField, ViewGridProps } from '../../types'; | ||||||||||||
import type { SetSelection } from '../../private-types'; | ||||||||||||
import { LAYOUT_GRID } from '../../constants'; | ||||||||||||
|
||||||||||||
interface GridItemProps< Item > { | ||||||||||||
selection: string[]; | ||||||||||||
|
@@ -54,10 +55,10 @@ function GridItem< Item >( { | |||||||||||
const id = getItemId( item ); | ||||||||||||
const isSelected = selection.includes( id ); | ||||||||||||
const renderedMediaField = mediaField?.render ? ( | ||||||||||||
<mediaField.render item={ item } /> | ||||||||||||
<mediaField.render item={ item } view={ LAYOUT_GRID } /> | ||||||||||||
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. Why adding a view to the render function is necessary? I'd like us to avoid this if possible. 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. Based on the layout, the field has a different UI: gutenberg/packages/fields/src/fields/featured-image/featured-image-view.tsx Lines 167 to 169 in 9cc6d80
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. Why? 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. Let me clarify: People are going to be able to register data views layouts independently from fields, meaning ideally the rendering of fields is universal and shouldn't be dependent on the layout type. 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. 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. We will have a similar issue even with the new layout. 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 know it's a difficult constraint, but I think we should think of this constraint as a good thing and instead try to solve it in a way that doesn't require knowledge about the layout. There seem to be one main difference:
So what does this difference mean for the "field" abstraction.
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'm going to proceed with this approach, given that the second one looks like not strictly related to this field control, and it needs further discussion. 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. Updated the PR with commit c2994e0 to no longer rely on the view prop. The main downside of this approach is that the click event is no longer configured for the grid view. So when the user clicks on the image, the editor isn't opened anymore:
Additionally, even though the field no longer relies on the view prop, setting the correct image dimensions still requires using the CSS cascade: 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 created a dedicated issue for this topic: #66336 |
||||||||||||
) : null; | ||||||||||||
const renderedPrimaryField = primaryField?.render ? ( | ||||||||||||
<primaryField.render item={ item } /> | ||||||||||||
<primaryField.render item={ item } view={ LAYOUT_GRID } /> | ||||||||||||
) : null; | ||||||||||||
return ( | ||||||||||||
<VStack | ||||||||||||
|
@@ -115,7 +116,10 @@ function GridItem< Item >( { | |||||||||||
key={ field.id } | ||||||||||||
className="dataviews-view-grid__field-value" | ||||||||||||
> | ||||||||||||
<field.render item={ item } /> | ||||||||||||
<field.render | ||||||||||||
item={ item } | ||||||||||||
view={ LAYOUT_GRID } | ||||||||||||
/> | ||||||||||||
</FlexItem> | ||||||||||||
); | ||||||||||||
} ) } | ||||||||||||
|
@@ -151,7 +155,10 @@ function GridItem< Item >( { | |||||||||||
className="dataviews-view-grid__field-value" | ||||||||||||
style={ { maxHeight: 'none' } } | ||||||||||||
> | ||||||||||||
<field.render item={ item } /> | ||||||||||||
<field.render | ||||||||||||
item={ item } | ||||||||||||
view={ LAYOUT_GRID } | ||||||||||||
/> | ||||||||||||
</FlexItem> | ||||||||||||
</> | ||||||||||||
</Flex> | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import clsx from 'clsx'; | |
*/ | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
import { decodeEntities } from '@wordpress/html-entities'; | ||
import { featuredImageField } from '@wordpress/fields'; | ||
import { | ||
createInterpolateElement, | ||
useMemo, | ||
|
@@ -33,11 +34,9 @@ import { useEntityRecords, store as coreStore } from '@wordpress/core-data'; | |
import { | ||
LAYOUT_GRID, | ||
LAYOUT_TABLE, | ||
LAYOUT_LIST, | ||
OPERATOR_IS_ANY, | ||
} from '../../utils/constants'; | ||
import { default as Link, useLink } from '../routes/link'; | ||
import Media from '../media'; | ||
import { default as Link } from '../routes/link'; | ||
|
||
// See https://github.com/WordPress/gutenberg/issues/55886 | ||
// We do not support custom statutes at the moment. | ||
|
@@ -81,46 +80,6 @@ const getFormattedDate = ( dateToDisplay ) => | |
getDate( dateToDisplay ) | ||
); | ||
|
||
function FeaturedImage( { item, viewType } ) { | ||
const isDisabled = item.status === 'trash'; | ||
const { onClick } = useLink( { | ||
postId: item.id, | ||
postType: item.type, | ||
canvas: 'edit', | ||
} ); | ||
const hasMedia = !! item.featured_media; | ||
const size = | ||
viewType === LAYOUT_GRID | ||
? [ 'large', 'full', 'medium', 'thumbnail' ] | ||
: [ 'thumbnail', 'medium', 'large', 'full' ]; | ||
const media = hasMedia ? ( | ||
<Media | ||
className="edit-site-post-list__featured-image" | ||
id={ item.featured_media } | ||
size={ size } | ||
/> | ||
) : null; | ||
const renderButton = viewType !== LAYOUT_LIST && ! isDisabled; | ||
return ( | ||
<div | ||
className={ `edit-site-post-list__featured-image-wrapper is-layout-${ viewType }` } | ||
> | ||
{ renderButton ? ( | ||
<button | ||
className="edit-site-post-list__featured-image-button" | ||
type="button" | ||
onClick={ onClick } | ||
aria-label={ item.title?.rendered || __( '(no title)' ) } | ||
> | ||
{ media } | ||
</button> | ||
) : ( | ||
media | ||
) } | ||
</div> | ||
); | ||
} | ||
|
||
function PostStatusField( { item } ) { | ||
const status = STATUSES.find( ( { value } ) => value === item.status ); | ||
const label = status?.label || item.status; | ||
|
@@ -190,15 +149,7 @@ function usePostFields( viewType ) { | |
|
||
const fields = useMemo( | ||
() => [ | ||
{ | ||
id: 'featured-image', | ||
label: __( 'Featured Image' ), | ||
getValue: ( { item } ) => item.featured_media, | ||
render: ( { item } ) => ( | ||
<FeaturedImage item={ item } viewType={ viewType } /> | ||
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. Two questions:
|
||
), | ||
enableSorting: false, | ||
}, | ||
featuredImageField, | ||
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 like that we're moving the dataviews fields to the fields package one by one 👍 and fixing them (removing dependencies...) |
||
{ | ||
label: __( 'Title' ), | ||
id: 'title', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.components-popover.components-dropdown__content.dataforms-layouts-panel__field-dropdown { | ||
z-index: z-index(".components-popover.components-dropdown__content.dataforms-layouts-panel__field-dropdown"); | ||
} |
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.
How this looks like in the regular panel? Can we update the storybook to see how fields of this type behave in both layouts?
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.
This change does not affect the appearance of the current fields in different views. The primary goal is to pass this information to the field, enabling it to adjust its layout based on the view property.