Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update to use an object instead of sizes string and fix grid size at max
  • Loading branch information
tellthemachines committed Jul 29, 2025
commit 792b887baf3e9810541f1c4b746551f491229ba5
12 changes: 9 additions & 3 deletions packages/dataviews/src/dataviews-layouts/grid/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ interface GridItemProps< Item > {
regularFields: NormalizedField< Item >[];
badgeFields: NormalizedField< Item >[];
hasBulkActions: boolean;
sizes: string;
mediaAppearance: {
maxImageWidth: string;
};
}

function GridItem< Item >( {
Expand All @@ -80,15 +82,19 @@ function GridItem< Item >( {
regularFields,
badgeFields,
hasBulkActions,
sizes,
mediaAppearance,
}: GridItemProps< Item > ) {
const { showTitle = true, showMedia = true, showDescription = true } = view;
const hasBulkAction = useHasAPossibleBulkAction( actions, item );
const id = getItemId( item );
const instanceId = useInstanceId( GridItem );
const isSelected = selection.includes( id );
const renderedMediaField = mediaField?.render ? (
<mediaField.render item={ item } field={ mediaField } sizes={ sizes } />
<mediaField.render
item={ item }
field={ mediaField }
mediaAppearance={ mediaAppearance }
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest for me, right now, this is way too advanced/complex for a small gain. I'd rather pass the "view" object here as a "temporary API". I can't imagine this "mediaAppearance" API sticking forever either.

I would like to understand better what other kind of "rendering config" we need per field before having a more formal/complex API.

Copy link
Contributor

Choose a reason for hiding this comment

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

But now, that I think about itrender is also called on DataForm which doesn't have the concept of "view". So my question here, what would be the simplest common denominator thing we could pass here that allow us to achieve the use-case without complexity. For instance I want to avoid resize observers as much as possible personally, can we instead having

config={ config }

with the following type

interface config {
   size?: "small" | "medium" | "large"
}

and each view could decide to pass the right "size" as needed.

  • For instance the "media" field in table would always pass "small"
  • The "media" field in grid would always pass "medium"
  • DataForm would probably pass "small" too

I realize this is close to what you're proposing but there are two changes:

  • Simpler because there's no need for resize observers
  • More generic because it's not just about "media", later we might want other kind of "rendering config" for fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simpler because there's no need for resize observers

The simpler config won't fix the need for resize observer, because that's already being used to build the grid layout. I just moved it to a different element because it isn't giving accurate numbers in its current location (the layout wrapper).

I did simplify the grid logic as much as I could so it's less dependent on breakpoints, but unfortunately we do still need to know the container size in order to display the right options in the preview size picker.

interface config {
size?: "small" | "medium" | "large"
}

small, medium, large won't really work here because the responsive image logic requires actual numeric values for its sizes attribute. The layout is responsible for setting the image size, so we should get that size from it. Otherwise we're putting the burden on the consumer to figure out what those sizes mean in px and produce a suitable number themselves, which might break if we ever change the layout.

We can call the object something more generic than mediaAppearance; it could be config or something else. But we do need those numeric widths for the responsive images to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated to a generic config with a size property that takes a string with the size in px.

/>
) : null;
const renderedTitleField =
showTitle && titleField?.render ? (
Expand Down
2 changes: 1 addition & 1 deletion packages/dataviews/src/dataviews-layouts/list/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function ListItem< Item >( {
<mediaField.render
item={ item }
field={ mediaField }
sizes="52px"
mediaAppearance={ { maxImageWidth: '52px' } }
/>
</div>
) : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function ColumnPrimary< Item >( {
<mediaField.render
item={ item }
field={ mediaField }
sizes="32px"
mediaAppearance={ { maxImageWidth: '32px' } }
/>
</div>
) }
Expand Down
4 changes: 3 additions & 1 deletion packages/dataviews/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ export type DataFormControlProps< Item > = {
export type DataViewRenderFieldProps< Item > = {
item: Item;
field: NormalizedField< Item >;
sizes?: string;
mediaAppearance?: {
maxImageWidth: string;
};
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { BasePost } from '../../types';

export const FeaturedImageView = ( {
item,
sizes = '',
mediaAppearance,
}: DataViewRenderFieldProps< BasePost > ) => {
const mediaId = item.featured_media;

Expand Down Expand Up @@ -42,7 +42,7 @@ export const FeaturedImageView = ( {
.join( ', ' )
: undefined
}
sizes={ sizes }
sizes={ mediaAppearance?.maxImageWidth || '100vw' }
/>
);
}
Expand Down