Skip to content
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.

Sign up for GitHub

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

Jump to bottom

DataViews: make mediaField not hidable #56643

Merged
merged 1 commit into from
Nov 30, 2023
Merged

DataViews: make mediaField not hidable #56643

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion packages/edit-site/src/components/dataviews/view-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ function PageSizeMenu( { view, onChangeView } ) {

function FieldsVisibilityMenu( { view, onChangeView, fields } ) {
const hidableFields = fields.filter(
( field ) => field.enableHiding !== false
( field ) =>
field.enableHiding !== false && field.id !== view.layout.mediaField
Copy link
Member Author

Choose a reason for hiding this comment

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

As a follow-up PR, I'd like to investigate what if we removed enableHiding from our API and favor the following logic: all fields are visible unless they are the view.layout.primaryField or view.layout.mediaField.

The primaryField config is only working on the grid layout type, but there is no reason it cannot be expanded to all layouts. For example, the side-by-side could use it to determine which field that triggers the "side by side" view (in addition to prevent hiding it).

Copy link
Contributor

Choose a reason for hiding this comment

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

all fields are visible unless they are the view.layout.primaryField or view.layout.mediaField.

I assume you mean all fields are hideable, right? If yes in the future we could have more fields that we don't want to be hideable, but still not the primary one. For example in a product title(primary), price not hideable.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's good, I'll leave it as it is.

Upon sleeping on it, I think what I don't like about the existing enableHiding is that is a view-concern that lives in the Field API. The same as maxWidth, etc. That may be fine when you only have a single view (TanStack table), but it's not necessarily good when you may have some – each may need different view tweaks.

Ok, so this PR is ready then :)

);
if ( ! hidableFields?.length ) {
return null;
Expand Down
Loading