WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.32k stars 4.12k forks source link

DataViews: stress testing `primaryField`, `mediaField`, and `view.fields` #65011

Open oandregal opened 1 week ago

oandregal commented 1 week ago

Description

I'd like to share some inconsistencies I've noticed with the primaryField and mediaField:

Step-by-step reproduction instructions

Layout With primaryField and mediaField Without primaryField and mediaField
Table Captura de ecrã 2024-09-03, às 18 20 18 Captura de ecrã 2024-09-03, às 18 20 11
Grid Captura de ecrã 2024-09-03, às 18 20 45 Captura de ecrã 2024-09-03, às 18 20 33
List Captura de ecrã 2024-09-03, às 18 21 00 Captura de ecrã 2024-09-03, às 18 20 55
oandregal commented 1 week ago

cc @youknowriad @jameskoster @jorgefilipecosta @ntsekouras for thoughts. This stems from this other conversation.

oandregal commented 1 week ago

Additionally, perhaps the "special slots" (primary/media) should be communicated in the property panel so users learn why they cannot be hidden.

This is something James touched upon here (ignore the combine fields logic for now, note the "Primary" in "Title"):

combining
youknowriad commented 1 week ago

I'm in the camp of treating all these "fields" properties the same and layout specific. In other words: I think the current layout property in view is to be removed and we should flatten everything and layouts shouldn't necessary keep the fields when you change.

I guess what I'm saying is that by trying to make all the layouts fit into the same model, we end up with a not ideal setup for any layout.

oandregal commented 1 week ago

I'm in the camp of treating all these "fields" properties the same and layout specific. In other words: I think the current layout property in view is to be removed and we should flatten everything and layouts shouldn't necessary keep the fields when you change.

I agree with the idea of having layout-specific logic.

However, I'm not sure what would be the actions steps after your comment (perhaps none?) 😅

I guess my question is: how do you judge the current situation? Specially the following items:

youknowriad commented 1 week ago

Media field not visible in properties panel while primary is.

Primary field is required as part of view.fields to be rendered in table layout, but not others. I guess your point here is that it's fine?

I'm not sure I follow here. I can see table layout working without any primary field.

Do we do something different when primary&media are not provided for grid/list or the current situation is ok?

I don't have a strong opinion here. Making them mandatory (rendering nothing) would be fine for me for these layouts. Current state is not ideal but also not that bad :P. I mean the developer will quickly figure out that in order for these layouts to work well, they need to provide a primary and media field.

ntsekouras commented 1 week ago

For primaryField I think it makes sense to keep it as a concept but for every layout (so part of the view) and every layout should handle as it seems fit(styles, position or whatever). DataViews should have at least one field that cannot be hidden and convey some important info for users to identify the entity they are seeing.

For me mediaField could also be removed from layout config and be top level key at view object, but optional. Then each view could handle the rendering if provided in a different way, but it shouldn't be mandatory, at least in our current views. I don't get why a grid view or the list view would require a mediaField. They both have value without one too.

If a specific (new) layout would require something it could just not render anything if not provided.

jameskoster commented 1 week ago

Making them mandatory (rendering nothing) would be fine for me for these layouts.

Is it possible to provide a fallback for mediaField rather than rendering nothing? It would be good to avoid this awkward overlap in grid layout when there's no thumbnail/preview:

Screenshot 2024-09-04 at 11 00 17

Similarly the placement and appearance of the actions menu looks very odd if primaryField is not rendered as noted in the OP. We should avoid that if possible. Perhaps we could fix by moving the actions button to the top right (similar to the checkbox):

Screenshot 2024-09-04 at 11 19 37

Agree with Nik that it should not be possible for users to configure options so that they end up with an empty table. I don't have a strong opinion about whether we use primaryField for that, or add some logic in view options to disable the ability to hide the last visible field, or both.

Practically, if a primaryField is supplied, it probably makes sense to force it to be visible? Hiding titles in the pages table isn't something I can ever imagine a user wanting to do.


Like table layout, I don't think it should be possible to configure List layout so that no fields are visible. Perhaps the same logic can be applied in Table and List layouts?