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.

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: Unify layout behavior. #67391

Open
youknowriad opened this issue Nov 28, 2024 · 8 comments · May be fixed by #67477
Open

DataViews: Unify layout behavior. #67391

youknowriad opened this issue Nov 28, 2024 · 8 comments · May be fixed by #67477
Assignees
Labels
[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@youknowriad
Copy link
Contributor

youknowriad commented Nov 28, 2024

Current the list, grid and table layouts work very differently when it comes to "fields". This is the result of multiple requests to achieve some designs. So we have:

  • combined fields support in table layouts
  • columns fields support in grid layouts
  • primary field in all of them
  • media field in some of them
  • badge fields in some of them

These differences created some friction and downsides:

  • Reordering fields is not consistent between layouts.
  • Not easy to toggle the visibility some fields when they're combined.
  • Designing a consistent ViewConfig dropdown is challenging.

I think it's time to take a look at the designs we achieved using these features and see whether we can do the same by altering our approach a little bit.

The main idea is to unify how all the layouts work with the following config:

{
   primaryField: "something"
   mediaField: "something"
   descriptionField: "something",
   otherFields: [ ]
}
  • The concept of "combined fields" is gone entirely. (at least not needed for our current use-cases, so we might as well remove it and see if we restore it later)
  • Table block renders a special column (the first one), with an opinionated design of primary + media + description fields combined.
  • The grid block columns config is not needed and replaced by the "description" field.
  • The list view is basically just the first column of the table layout (additional fields can be rendered after it)
  • The rest of the fields are an ordered list of simple fields.
  • The special fields (primary, media, description) are togglable: you can hide/show them but you can't reorder them. Some layouts might make some of these mandatory.
  • There's still a need of some kind of "formats" support for fields to support token fields or having the ability to show/hide field labels.

Related #57596 #58012

@youknowriad youknowriad added [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Enhancement A suggestion for improvement. labels Nov 28, 2024
@oandregal
Copy link
Member

oandregal commented Nov 28, 2024

By creating a "special bundle" of primary/media/description, this also simplifies conceptually what's the "main click target" of the layout. It was perhaps already clear in grid and list, but not in table — anything could be a target there.

This situation with table has created issues and forced us to add a onClick / isItemClickable API to DataViews tied to the primary/media field. I think we can further simplify this, remove the props from DataViews and do the following:

{
   primaryField: "something"
   mediaField: "something"
   descriptionField: "something",
   onClick: "action",
   otherFields: [ ]
}

The layout declares which action will be triggered upon clicking primary/media fields. In the list view, this action would be the one that's displayed — rather than just displaying the first one as we currently do.

@lsl
Copy link
Contributor

lsl commented Nov 28, 2024

Table block renders a special column (the first one), with an opinionated design of primary + media + description fields combined.

Could this be made optional and dependent on providing a primary field?

It'd also be good if the media and description fields were optional.

onClick: "action",

This would simplify things but I would also need a way to disable it for certain rows. Return early in the callback works ok but it'd be better if the on click hover/cursor styling was correct too.

Funnily enough, I happen to be working on a concrete example where this proposed onClick behavior would solve a bunch of styling problems: https://github.com/Automattic/wp-calypso/pull/96901/files?w=1#diff-57ab38ed1e7c74bc864cf18dfbc8ec875063090044d686b459bf7beace984fa4R10-R14

@youknowriad
Copy link
Contributor Author

Could this be made optional and dependent on providing a primary field?

I guess if you provide only regular fields and don't assign any primary/media nor description field, the first column won't be rendered here. That said, I would love to understand more the use-cases where there's no "primary column". Could you share a bit more?

onClick: "action",

Personally I don't think the "onClick" action should be part of the "view" object. The view object should always stay a serializable object that you can persist and share in the backend.

This would simplify things but I would also need a way to disable it for certain rows.

If you need something like that, I think it's always a good idea to share the use-case so we can understand better what's the right solution for it.

@ntsekouras
Copy link
Contributor

The grid block columns config is not needed and replaced by the "description" field.

What do you mean here?

@youknowriad
Copy link
Contributor Author

youknowriad commented Nov 29, 2024

@ntsekouras The screenshot above is the only place where we currently use the "columns" config. We can easily remove that by making the "description" a special field to achieve the same result. We may also remove that unnecessary "description" label in the same change.

@youknowriad youknowriad linked a pull request Dec 2, 2024 that will close this issue
6 tasks
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 2, 2024
@lsl
Copy link
Contributor

lsl commented Dec 3, 2024

I would love to understand more the use-cases where there's no "primary column". Could you share a bit more?

Just thinking generally, if there are going to be requirements on this field like not being able to reorder it, then there might be scenarios where you just want to display a table of data.

disable on click for certain rows.

If you need something like that, I think it's always a good idea to share the use-case so we can understand better what's the right solution for it.

WordPress.com/sites dashboard, deleted sites in the list of sites need to be shown but they don't have on any click behavior, only a restore action.

@youknowriad
Copy link
Contributor Author

WordPress.com/sites dashboard, deleted sites in the list of sites need to be shown but they don't have on any click behavior, only a restore action.

Makes sense there's an isItemClickable prop already to support this use-case.

@lsl
Copy link
Contributor

lsl commented Dec 4, 2024

Makes sense there's an isItemClickable prop already to support this use-case.

Ah, thanks, looks like we need to update 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants