DataViews: add details for pages in list layout #57523
Conversation
|
Size Change: +635 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
oandregal
added
[Type] Experimental
oandregal
requested review from
jameskoster,
youknowriad,
ntsekouras and
jorgefilipecosta
| onChangeView={ onChangeView } | ||
| onSelectionChange={ onSelectionChange } | ||
| /> | ||
| { showDetails && <Details item={ pageRecord } /> } |
There was a problem hiding this comment.
I wish I could reuse the existing "details" components instead of having to create Details, but the current ones do too many things (assume they are rendered in the sidebar, deal with router logic, fetch data, etc.). We should revisit when making this stable.
| { stripHTML( excerpt ) } | ||
| </Truncate> | ||
| ) } | ||
| <PageDetails id={ item.id } /> |
There was a problem hiding this comment.
I don't very much like reusing PageDetails here. It handles things like data retrieval that we don't need here because we already have the record. However, I thought I wouldn't play catch-up with the existing PageDetails in case it's modified, so I went for reusing it.
oandregal
changed the title
| @@ -31,6 +31,7 @@ export default function DataViews( { | |||
| paginationInfo, | |||
| supportedLayouts, | |||
| onSelectionChange, | |||
| onDetailsChange = null, | |||
There was a problem hiding this comment.
This feels to me just part of the list view.. Do you think it will have some kind of value in other views too? Maybe it's something that we should render in a field based on the view type? Maybe part of the layoutConfig?
There was a problem hiding this comment.
I did consider a different implementation: make it an action. From a conceptual point of view, it is an action. I looked into this, but I wasn't convinced it was worth it: it required connecting the "actions" and "view" APIs. The details only require a callback, everything else is provided by the list view already, so I thought of making that explicit.
I haven't considered implementing this as part of the fields or view APIs, but happy to hear your thoughts.
|
Nice work so far. I pushed a couple of style adjustments for list view (including a fix for the title truncation) and the details panel. A couple of items to consider, acknowledging some may be follow-up territory:
|
|
To be honest, I'm not sure we should do this now. I think this is going to set us on the wrong path on how to deal with this. Basically, I think the "details" page should actually be just the existing pages we have and not create new ones. But in order to do this, we need first to merge both "page lists": "browse all" and the regular "pages" page. So IMO, we should not implement this PR now and wait until we used the side by side views as a replacement for the parent page as otherwise, we'll be adding complexity and duplicating UIs for nothing. |
What do you mean by this? Or perhaps this is better for clarification: from an interaction point of view, when a user clicks on the details button, what's the expected result for you (1 or 2)?
If it is 2, we wouldn't be able to reuse the existing
Yeah, this was my next step, happy to do it before this one. |
Sure, I can look into this tomorrow.
This actually reminds me that I wanted to look at focus: move it to the "close/back" button when the details are opened. I'll take a look at this tomorrow.
This is connected to Riad's point above. I expect this to happen: when you go to the URL for a specific page (e.g.:
I'd like to address this as a follow-up. |
I expect that ultimately it's going to look like 2 but I expect 1 and 2 to be the exact same page. In other words, I expect the click to just do So personally If I had to implement something today for the "details" click (before actually moving the list view up), I'd just navigate to 1 and leave the refactor of the details page for post merge. |
I agree. It's related to what I mentioned to Jay #57523 (comment)
This works for me as well as a first step. Thanks for the suggestion. It does make the interaction a bit clunky because going back to where you were (dataviews), you'll have to go to "Manage all pages > switch to list view", etc. It's too big a step. Though it's temporary. If it works for people, I'll do this and then immediately look into substituting the root page. |
Prepared #57578 and marked this PR as draft. |
|
Related PR: use the pages-dataviews as the default for the "Pages" section. #57589 |
|
Trying to land this in pieces is creating some weird experiences. Implemented everything together at #57578 It's not too big a PR anyway. |
|
#57578 has landed, so closing this one. |
Part of #55083
Superseded by #57578
What?
Implement the page details on the list view.
Gravacao.do.ecra.2024-01-04.as.12.16.19.mov
Why?
It's part of the design spec:
How?
This PR introduces the details as a consumer action. They render the details, like they handle the actions and the preview.
onDetailsChangecallback, that signals the user triggered the details for an item.Pagecomponent is updated to take anonClosecallback. If present, it renders a "left chevron" button, otherwise, it's not present.Alternatives
I considered making DataViews handling the details screen. This would require it to absorb parts of the site editor UI that are now outside the library, so I decided against it:
Testing Instructions
Test pages show the details:
Test the templates do not have the details button:
Follow-ups